Mark all cccl and cuco kernels with hidden visibility #523
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
robertmaynard
added
bug
robertmaynard
force-pushed
the
bug/mark_cccl_cuco_kernels_as_hidden
branch
from
5ff3398 to
7eab859
Compare
Closed
3 tasks
bdice
approved these changes
Jan 16, 2024
|
Looks good to me. We can merge this after testing at least a few of RAFT, cuML, cuGraph, etc. |
robertmaynard
force-pushed
the
bug/mark_cccl_cuco_kernels_as_hidden
branch
2 times, most recently
from
c04724c to
45c8ad8
Compare
robertmaynard
force-pushed
the
bug/mark_cccl_cuco_kernels_as_hidden
branch
from
45c8ad8 to
2fca2d6
Compare
3 tasks
RAFT PR: rapidsai/raft#2098 builds are good |
|
I think the valid builds in raft/cudf/cugraph are sufficient that this PR won't break anything. |
|
/merge |
PointKernel
pushed a commit
to PointKernel/rapids-cmake
that referenced
this pull request
Jan 23, 2024
Patch both CCCL and CUCO to have only internal linkage. For cuco I am working on upstreaming these changes ( NVIDIA/cuCollections#422 ). Once that is accepted and we have validated that moving cuco is stable ( e.g. changes around `cuco::experimental::static_set` ) we can drop this patch set. For cccl the long term fix is to move to CCCL 2.3+, but due to issues ( NVIDIA/cccl#1249, maybe others ) that isn't viable for the 24.02 timeframe. Since the CCCL changes mean C++ and CUDA sources have non compatible ABI's, we need to specify `THRUST_DISABLE_ABI_NAMESPACE` and `THRUST_IGNORE_ABI_NAMESPACE_ERROR` so that we don't change ABI in rapids-cmake consumers since they expect 2.2 behavior. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Bradley Dice (https://github.com/bdice) URL: rapidsai#523
This was referenced Mar 5, 2024
rapids-bot bot
pushed a commit
to rapidsai/cudf
that referenced
this pull request
Mar 5, 2024
This is to remove the row conversion code from libcudf. It was move from spark-rapids-jni (by #14664) to temporarily workaround the issue due to conflict of kernel names that causes invalid memory access when calling to `thrust::in(ex)clusive_scan` (NVIDIA/spark-rapids-jni#1567). Now we have fixes for the namespace visibility issue (by marking all libcudf kenels private in rapidsai/rapids-cmake#523 and NVIDIA/cuCollections#422) and need to move back the code. Closes #14853. Authors: - Nghia Truong (https://github.com/ttnghia) Approvers: - David Wendt (https://github.com/davidwendt) - Bradley Dice (https://github.com/bdice) URL: #15234
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.
Description
Patch both CCCL and CUCO to have only internal linkage.
For cuco I am working on upstreaming these changes ( NVIDIA/cuCollections#422 ). Once that is accepted and we have validated that moving cuco is stable ( e.g. changes around
cuco::experimental::static_set) we can drop this patch set.For cccl the long term fix is to move to CCCL 2.3+, but due to issues ( NVIDIA/cccl#1249, maybe others ) that isn't viable for the 24.02 timeframe.
Since the CCCL changes mean C++ and CUDA sources have non compatible ABI's, we need to specify
THRUST_DISABLE_ABI_NAMESPACEandTHRUST_IGNORE_ABI_NAMESPACE_ERRORso that we don't change ABI in rapids-cmake consumers since they expect 2.2 behavior.Checklist