Patch cccl and cuco so that all CUDA kernels they provide have internal linkage #14735
Conversation
robertmaynard
added
bug
github-actions
bot
added
libcudf
robertmaynard
force-pushed
the
bug/mark_cccl_cuco_kernels_as_hidden
branch
from
439ffdc to
dd165b0
Compare
There was a problem hiding this comment.
This seems fine to me. @robertmaynard Can you expand a bit more on these questions before I approve:
-
How this was tested/validated with Spark-RAPIDS?
Since the CCCL changes mean C++ and CUDA sources have non compatible ABI's, we need to specify
THRUST_DISABLE_ABI_NAMESPACE.
To clarify, "the CCCL changes" are from the patch in this PR, and they are the same namespace changes as in 2.3.0? (In other words, this patch is a partial backport of 2.3.0?)
If these patches work for cudf CI, I believe we can move them to rapids-cmake and have the rapids_cpm_cccl package automatically provide them.
This PR's CI is passing. Will we be applying these same CCCL/cuCollections patches to the rapids-cmake CCCL/cuCollections? Do you want to open that PR and test it on other RAPIDS repos (e.g. RAFT, cuML, cuGraph) before merging this one?
|
|
Closing as rapidsai/rapids-cmake#523 has been merged |
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 currently. If these patches work for cudf CI, I believe we can move them to rapids-cmake and have the
rapids_cpm_ccclpackage automatically provide them.Since the CCCL changes mean C++ and CUDA sources have non compatible ABI's, we need to specify
THRUST_DISABLE_ABI_NAMESPACE. Likewise we need to specifyTHRUST_IGNORE_ABI_NAMESPACE_ERRORso that we don't change our ABI ( since it depends on thrust types ).Checklist