-
Notifications
You must be signed in to change notification settings - Fork 915
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
Update to CCCL 2.7.0-rc2. #17233
Update to CCCL 2.7.0-rc2. #17233
Conversation
… due to code changes in thrust. This adopts those patches to apply with CCCL 2.6. Should only be merged when updating the CCCL version
cpp/cmake/thirdparty/get_cccl.cmake
Outdated
# TODO: Does this work correctly? Is there a better way to set this? Should | ||
# CCCL be setting THRUST_DISPATCH_TYPE with "FORCE"? | ||
set(THRUST_DISPATCH_TYPE "Force32bit" STRING "Thrust offset type dispatch") |
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.
@robertmaynard I'd appreciate your eyes on this. There was an upstream change in CCCL in NVIDIA/cccl#2310 that made it possible to force CCCL to only compile kernels with 32-bit offsets when THRUST_DISPATCH_TYPE
is set to "Force32bit"
. However, I do not know how to forward this argument through rapids_cpm_cccl
. If I were using rapids_cpm_find
I think I could use OPTIONS
. What's the best approach?
Secondly, I am wondering if CCCL should be using FORCE
here: https://github.com/NVIDIA/cccl/blob/264b587d0621ee2c6bec27aa015078056cb5f021/thrust/CMakeLists.txt#L38
Does that override my ability to set the variable ahead of time, or not since it's marked as CACHE
?
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.
Yes the FORCE
means that we can't overwrite as each time CMake is invoked the value will be reset to Force32bit
.
As for passing this down from rapids_cpm_cccl
you would do rapids_cpm_cccl(CPM_ARGS OPTIONS "THRUST_DISPATCH_TYPE Force32bit")
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.
NVIDIA/cccl#2844 exposed the dispatch mode in thrust_create_target
but I don't see how we are calling that in rapids-cmake. For now I am going to try and move forward with 2.7.0-rc2
as-is rather than trying to backport this with a patch. We can adopt that dispatch option in a later CCCL update.
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.
Reverted ce7e991 for now.
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
/ok to test |
This is ready for review by cuDF CMake codeowners. |
This PR updates to CCCL 2.7.0-rc2. Do not merge until all of RAPIDS is ready to update.
Depends on rapidsai/rapids-cmake#710 and should be admin-merged immediately after that PR.
Part of rapidsai/build-planning#115.