-
Notifications
You must be signed in to change notification settings - Fork 198
fix(visibility): add symbol visibility controls to cuvs #2101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7f3c596
7118e80
9a6cde7
ae178ef
c956741
61b1049
528edaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #pragma once | ||
|
|
||
| // Wrapper for cuda_fp16.h that ensures __half gets default symbol visibility. | ||
| // | ||
| // GCC's "type visibility" rule causes template instantiations over __half to | ||
| // inherit hidden visibility when -fvisibility=hidden is in effect, because | ||
| // __half is a user-defined type first seen under hidden visibility. By | ||
| // including cuda_fp16.h under #pragma GCC visibility push(default), the __half | ||
| // type acquires default visibility, and downstream template instantiations | ||
| // (e.g., index<__half, ...>) will be properly exported from shared libraries. | ||
| #pragma GCC visibility push(default) | ||
| #include <cuda_fp16.h> // NOLINT | ||
| #pragma GCC visibility pop |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include <cstdint> | ||
|
|
||
| #include "../ivf_common.cuh" | ||
| #include "../ivf_list.cuh" | ||
| #include "ivf_flat_helpers.cuh" | ||
| #include <cuvs/neighbors/ivf_flat.hpp> | ||
|
|
||
|
|
@@ -206,3 +207,15 @@ void recompute_internal_state(const raft::resources& res, index<uint8_t, int64_t | |
| } | ||
|
|
||
| } // namespace cuvs::neighbors::ivf_flat::helpers | ||
|
|
||
| namespace cuvs::neighbors::ivf { | ||
|
|
||
| template CUVS_EXPORT void | ||
| resize_list<list<cuvs::neighbors::ivf_flat::list_spec, uint32_t, half, int64_t>>( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this a missing instantiation?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is related to your other question. nvcc + template specialization + explicit instantiation + explicit symbol visibility markup hit a number of edge cases and we had to find the magic incantation that would both be supported by nvcc and result in the right output. I won't pretend that I was particularly principled in my choice, I just tried the combinations that were closest to standards-compliant in reverse order until one produced the outcome I wanted. |
||
| raft::resources const&, | ||
| std::shared_ptr<list<cuvs::neighbors::ivf_flat::list_spec, uint32_t, half, int64_t>>&, | ||
| const list<cuvs::neighbors::ivf_flat::list_spec, uint32_t, half, int64_t>::spec_type&, | ||
| list<cuvs::neighbors::ivf_flat::list_spec, uint32_t, half, int64_t>::size_type, | ||
| list<cuvs::neighbors::ivf_flat::list_spec, uint32_t, half, int64_t>::size_type); | ||
|
|
||
| } // namespace cuvs::neighbors::ivf | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have to manually mark templated functions for export?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was basically an nvcc bug/quirk where it wouldn't accept the attribute in the same places that gcc would. Specifically I believe it was when there was a templated function that we wanted to export, the markup on the namespace wasn't affecting the symbol so we had to put it on individual symbols. I'd say more quirk than bug since I don't believe there's any standard requiring the attribute on the namespace to work, gcc just happens to support it.