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 patches for CCCL 2.6 #16668

Closed
wants to merge 1 commit into from

Conversation

miscco
Copy link
Contributor

@miscco miscco commented Aug 27, 2024

This adopts the patches to thrust to apply with CCCL 2.6.

Should only be merged when updating the CCCL version, but I am opening them now to not forget it and then accidentally drop them

… 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
@miscco miscco requested a review from a team as a code owner August 27, 2024 12:19
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Aug 27, 2024
+ status = call arguments; \
+ } \
+ else \
+ { \
+ throw std::runtime_error("THRUST_INDEX_TYPE_DISPATCH 64-bit count is unsupported in libcudf"); \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our formatter did some crazy things here, so that the patches are less localized. Shout if you need them to be minimal

@davidwendt davidwendt changed the title Update CCCL patches Update patches for CCCL 2.6 Aug 27, 2024
@jrhemstad
Copy link
Contributor

Better yet would be to just nuke these patches. I believe we can do that once these two CCCL issues are done:

@vyasr
Copy link
Contributor

vyasr commented Sep 26, 2024

It looks like both of the above are merged now (NVIDIA/cccl#2154 just merged yesterday) so in principle we should be able to drop these patches if we upgrade straight to CCCL>=2.6.2.

@bdice
Copy link
Contributor

bdice commented Sep 30, 2024

@jrhemstad @vyasr We have three patches, and I see two fixes. I see how the CCCL offset-dispatch changes will fix thrust_disable_64bit_dispatching.diff and ChainedPolicy changes will fix thrust_faster_scan_compile_times.diff.

I don't see a solution for the #pragma unroll 1 in thrust_faster_sort_compile_times.diff yet. Is that correct? Am I missing something?

It'd be nice to update CCCL and drop all patches at once, but it'd be okay to get rid of two if we aren't able to solve the third one in the near term.

@vyasr
Copy link
Contributor

vyasr commented Oct 1, 2024

That's a good question. I didn't investigate and was just responding to the comments. If we have to keep one patch around that's not the end of the world, although if that's the case given that the other bits were resolved upstream we should open a CCCL issue requesting the change to sort as well.

@davidwendt
Copy link
Contributor

We have moved to CCCL 2.7 so perhaps this can be closed?

@bdice
Copy link
Contributor

bdice commented Dec 13, 2024

Yes, I included the relevant parts in #17233. Thanks for the help! Closing.

@bdice bdice closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants