Use cuda::proclaim_return_type on device lambdas. - #14577
Merged
Conversation
github-actions
Bot
added
libcudf
bdice
added
improvement
bdice
marked this pull request as ready for review
December 5, 2023 22:25
ttnghia
approved these changes
Dec 6, 2023
davidwendt
reviewed
Dec 7, 2023
davidwendt
reviewed
Dec 7, 2023
davidwendt
reviewed
Dec 7, 2023
davidwendt
reviewed
Dec 7, 2023
davidwendt
approved these changes
Dec 8, 2023
Contributor
Author
|
/merge |
vuule
reviewed
Dec 8, 2023
vuule
left a comment
vuule
left a comment
Contributor
There was a problem hiding this comment.
never finished the review :(
Wanted to asked about this comment anyway.
Comment on lines
+482
to
+483
| // Needed so that the first instance of the implicit destructor for any TU isn't 'constructed' | ||
| // from a host+device function marking the implicit version also as host+device |
Contributor
Author
There was a problem hiding this comment.
That's a comment we acquired with help from @robertmaynard. I was suffering from inexplicable compilation problems and this fixed it.
ttnghia
reviewed
Dec 8, 2023
| ~mutable_column_view(){ | ||
| // Needed so that the first instance of the implicit destructor for any TU isn't 'constructed' | ||
| // from a host+device function marking the implicit version also as host+device | ||
| }; |
Contributor
There was a problem hiding this comment.
This will trigger IDE warning.
Suggested change
| }; | |
| } |
26 tasks
karthikeyann
pushed a commit
to karthikeyann/cudf
that referenced
this pull request
Dec 12, 2023
This PR makes cudf compatible with Thrust 2 by adding `cuda::proclaim_return_type`, but does not upgrade the version of Thrust/CCCL just yet. Currently we use libcudacxx 2.1.0, which makes `cuda::proclaim_return_type` available, but we still use Thrust 1.17.2 which doesn't require device lambdas to have proclaimed return types. This diff is separated out from NVIDIA#14576 and reduces the diff we must carry in NVIDIA#14576 -- which should contain only packaging changes / version updates for the CCCL 2 migration. This PR is **nonbreaking**, while NVIDIA#14576 will be **breaking**. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Nghia Truong (https://github.com/ttnghia) - David Wendt (https://github.com/davidwendt) URL: NVIDIA#14577
Merged
3 tasks
raydouglass
pushed a commit
that referenced
this pull request
Dec 19, 2023
This PR updates cuDF to CCCL 2.2.0. Do not merge until all of RAPIDS is ready to update. Depends on #14577. Replaces #13222. Authors: - Bradley Dice (https://github.com/bdice) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Ray Douglass (https://github.com/raydouglass) - Vyas Ramasubramani (https://github.com/vyasr)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR makes cudf compatible with Thrust 2 by adding
cuda::proclaim_return_type, but does not upgrade the version of Thrust/CCCL just yet. Currently we use libcudacxx 2.1.0, which makescuda::proclaim_return_typeavailable, but we still use Thrust 1.17.2 which doesn't require device lambdas to have proclaimed return types. This diff is separated out from #14576 and reduces the diff we must carry in #14576 -- which should contain only packaging changes / version updates for the CCCL 2 migration. This PR is nonbreaking, while #14576 will be breaking.Checklist