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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Dec 4, 2024
Merged

Update to CCCL 2.7.0-rc2. #17233

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3b1d74d
Two of the patches that we need for thrust cannot be applied CCCL 2.6…
miscco Aug 27, 2024
39e1655
Build with CCCL 2.7.0-rc2.
bdice Nov 1, 2024
f6d6441
Merge remote-tracking branch 'upstream/branch-24.12' into cccl-2.7.0-rc2
bdice Nov 13, 2024
e231fa3
Merge branch 'adopt_cccl_patches_2_6' into cccl-2.7.0-rc2
bdice Nov 13, 2024
732040f
Drop cccl_symbol_visibility patch.
bdice Nov 13, 2024
ce7e991
Remove patch for 64-bit dispatching.
bdice Nov 13, 2024
f3b6a49
Revert "Remove patch for 64-bit dispatching."
bdice Nov 25, 2024
7a9c04a
Merge branch 'branch-24.12' into cccl-2.7.0-rc2
bdice Nov 25, 2024
53e93c4
Merge branch 'branch-25.02' into cccl-2.7.0-rc2
bdice Nov 25, 2024
e6c98b1
Merge remote-tracking branch 'upstream/branch-25.02' into cccl-2.7.0-rc2
bdice Dec 3, 2024
650ab50
Revert testing changes.
bdice Dec 4, 2024
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
View file
Open in desktop
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")
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")

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted ce7e991 for now.

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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

25 changes: 0 additions & 25 deletions cpp/cmake/thirdparty/patches/thrust_disable_64bit_dispatching.diff
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions cpp/cmake/thirdparty/patches/thrust_faster_sort_compile_times.diff
View file
Open in desktop
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
View file
Open in desktop
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)

bdice marked this conversation as resolved.
Show resolved Hide resolved
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