Inline get_cache_idx #1492
Merged
Inline get_cache_idx #1492
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I cannot reproduce the linker error. Building with clang 11.1 and nvcc
11.8. Both test files have raft::cache::(anonymous
namespace)::get_cache_idx defined though..
$ nm -g -C cpp/build/CMakeFiles/UTILS_TEST.dir/test/util/get_cache_idx_1.cu.o | grep get_cache_idx
U __cudaRegisterLinkedBinary_667dac6a_18_get_cache_idx_1_cu_eeb3336e_3832672
0000000000000000 D __fatbinwrap_667dac6a_18_get_cache_idx_1_cu_eeb3336e_3832672
0000000000000010 T __device_stub__ZN4raft5cache59_GLOBAL__N__667dac6a_18_get_cache_idx_1_cu_eeb3336e_383267213get_cache_idxEPiiS2_iiS2_S2_Pbi(int*, int, int*, int, int, int*, int*, bool*, int)
0000000000000000 T raft::cache::(anonymous namespace)::get_cache_idx(int*, int, int*, int, int, int*, int*, bool*, int)
$ nm -g -C cpp/build/CMakeFiles/UTILS_TEST.dir/test/util/get_cache_idx_2.cu.o | grep get_cache_idx
U __cudaRegisterLinkedBinary_74c80384_18_get_cache_idx_2_cu_a658dc80
0000000000000000 D __fatbinwrap_74c80384_18_get_cache_idx_2_cu_a658dc80
0000000000000010 T __device_stub__ZN4raft5cache51_GLOBAL__N__74c80384_18_get_cache_idx_2_cu_a658dc8013get_cache_idxEPiiS2_iiS2_S2_Pbi(int*, int, int*, int, int, int*, int*, bool*, int)
0000000000000000 T raft::cache::(anonymous namespace)::get_cache_idx(int*, int, int*, int, int, int*, int*, bool*, int)
Now we do get linker errors
This reverts commit 730b8a5.
cjnolet
approved these changes
May 5, 2023
|
It seems to be working! Thanks. |
|
/merge |
rapids-bot bot
pushed a commit
that referenced
this pull request
Aug 11, 2023
Fixes issue #1511. Make get_cache_idx a weak symbol (to allow linking multiple symbols) without marking it inline (to avoid compilation warnings that are promoted to errors in nvcc 12). Related issues: - #1490 - #1722 Related PRs: - #1732 - #1492 Authors: - Allard Hendriksen (https://github.com/ahendriksen) - Artem M. Chirkin (https://github.com/achirkin) Approvers: - Artem M. Chirkin (https://github.com/achirkin) - Corey J. Nolet (https://github.com/cjnolet) URL: #1733
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to issue #1490.
The
get_cache_idxkernel is currently defined in an anonymous namespace. In #1490, it was reported that this could lead to linker errors in CUDA 11.4 in combination with (presumably?) clang.I tried to reproduce the linker error in commit 01cf409 but could not, as described in the commit message.
This PR attempts to fix the linker errors by marking
get_cache_idxas inline and by removing the anonymous namespace.