Skip to content
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

Draft
wants to merge 6 commits into
base: branch-24.12
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cpp/cmake/thirdparty/get_cccl.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ function(find_and_configure_cccl)
)

# Find or install CCCL with our custom set of patches
# 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")
Comment on lines +37 to +39
Copy link
Contributor Author

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?

Copy link
Contributor

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")

rapids_cpm_cccl(BUILD_EXPORT_SET cudf-exports INSTALL_EXPORT_SET cudf-exports)

endfunction()
Expand Down
10 changes: 0 additions & 10 deletions cpp/cmake/thirdparty/patches/cccl_override.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,6 @@
"packages" : {
"CCCL" : {
"patches" : [
{
"file" : "${current_json_dir}/cccl_symbol_visibility.diff",
"issue" : "Correct symbol visibility issues in libcudacxx [https://github.com/NVIDIA/cccl/pull/1832/]",
"fixed_in" : "2.6"
},
{
"file" : "${current_json_dir}/thrust_disable_64bit_dispatching.diff",
"issue" : "Remove 64bit dispatching as not needed by libcudf and results in compiling twice as many kernels [https://github.com/rapidsai/cudf/pull/11437]",
"fixed_in" : ""
},
{
"file" : "${current_json_dir}/thrust_faster_sort_compile_times.diff",
"issue" : "Improve Thrust sort compile times by not unrolling loops for inlined comparators [https://github.com/rapidsai/cudf/pull/10577]",
Expand Down
27 changes: 0 additions & 27 deletions cpp/cmake/thirdparty/patches/cccl_symbol_visibility.diff

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
diff --git a/cub/cub/block/block_merge_sort.cuh b/cub/cub/block/block_merge_sort.cuh
index eb76ebb0b..c6c529a50 100644
index 29510db5e..cf57e5786 100644
--- a/cub/cub/block/block_merge_sort.cuh
+++ b/cub/cub/block/block_merge_sort.cuh
@@ -95,7 +95,7 @@ _CCCL_DEVICE _CCCL_FORCEINLINE void SerialMerge(
KeyT key1 = keys_shared[keys1_beg];
KeyT key2 = keys_shared[keys2_beg];

-#pragma unroll
+#pragma unroll 1
for (int item = 0; item < ITEMS_PER_THREAD; ++item)
{
bool p = (keys2_beg < keys2_end) && ((keys1_beg >= keys1_end) || compare_op(key2, key1));
@@ -376,7 +376,7 @@ public:
const bool p = (keys2_beg < keys2_end) && ((keys1_beg >= keys1_end) || compare_op(key2, key1));
@@ -374,7 +374,7 @@ public:
//
KeyT max_key = oob_default;

-#pragma unroll
+#pragma unroll 1
for (int item = 1; item < ITEMS_PER_THREAD; ++item)
Expand All @@ -27,7 +27,7 @@ index 7d9e8622f..da5627306 100644
@@ -87,10 +87,10 @@ StableOddEvenSort(KeyT (&keys)[ITEMS_PER_THREAD], ValueT (&items)[ITEMS_PER_THRE
{
constexpr bool KEYS_ONLY = ::cuda::std::is_same<ValueT, NullType>::value;

-#pragma unroll
+#pragma unroll 1
for (int i = 0; i < ITEMS_PER_THREAD; ++i)
Expand Down
4 changes: 4 additions & 0 deletions rapids_config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
# or implied. See the License for the specific language governing permissions and limitations under
# the License.
# =============================================================================

set(rapids-cmake-repo bdice/rapids-cmake)
set(rapids-cmake-branch cccl-2.7.0-rc2)

file(READ "${CMAKE_CURRENT_LIST_DIR}/VERSION" _rapids_version)
if(_rapids_version MATCHES [[^([0-9][0-9])\.([0-9][0-9])\.([0-9][0-9])]])
set(RAPIDS_VERSION_MAJOR "${CMAKE_MATCH_1}")
Expand Down
Loading