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

Use cuda::proclaim_return_type on device lambdas. #14577

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Dec 5, 2023

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 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 #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

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. Java Affects Java cuDF API. labels Dec 5, 2023
@bdice bdice added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 5, 2023
@bdice bdice marked this pull request as ready for review December 5, 2023 22:25
@bdice bdice requested review from a team as code owners December 5, 2023 22:25
@bdice bdice requested review from robertmaynard and vuule December 5, 2023 22:25
cpp/src/copying/copy.cu Outdated Show resolved Hide resolved
@bdice bdice mentioned this pull request Dec 8, 2023
3 tasks
@bdice bdice requested a review from davidwendt December 8, 2023 18:20
@bdice bdice self-assigned this Dec 8, 2023
@bdice
Copy link
Contributor Author

bdice commented Dec 8, 2023

/merge

Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor

Choose a reason for hiding this comment

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

wat :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a comment we acquired with help from @robertmaynard. I was suffering from inexplicable compilation problems and this fixed it.

~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
};
Copy link
Contributor

Choose a reason for hiding this comment

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

This will trigger IDE warning.

Suggested change
};
}

@rapids-bot rapids-bot bot merged commit 7b551ef into rapidsai:branch-24.02 Dec 8, 2023
67 checks passed
@bdice bdice mentioned this pull request Dec 8, 2023
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 rapidsai#14576 and reduces the diff we must carry in rapidsai#14576 -- which should contain only packaging changes / version updates for the CCCL 2 migration. This PR is **nonbreaking**, while rapidsai#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: rapidsai#14577
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants