[libcu++] Deprecate default stream_ref constructor and fix some few last usages - #5310
Merged
pciolkosz merged 5 commits intoJul 22, 2025
Merged
Conversation
Contributor
ðĻ CI finished in 1h 50m: Pass: 77%/205 | Total: 3d 18h | Avg: 26m 27s | Max: 1h 46m | Hits: 84%/293547
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| CCCL Packaging | |
| +/- | libcu++ |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| +/- | CCCL Packaging |
| +/- | libcu++ |
| +/- | CUB |
| +/- | Thrust |
| +/- | CUDA Experimental |
| +/- | stdpar |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
ðâ Runner counts (total jobs: 205)
| # | Runner |
|---|---|
| 128 | linux-amd64-cpu16 |
| 23 | windows-amd64-cpu16 |
| 14 | linux-amd64-gpu-h100-latest-1 |
| 14 | linux-amd64-gpu-rtxa6000-latest-1 |
| 12 | linux-arm64-cpu16 |
| 11 | linux-amd64-gpu-rtx2080-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
miscco
approved these changes
Jul 18, 2025
| // Query relevant properties from the environment | ||
| auto stream = _CUDA_STD_EXEC::__query_or(env, ::cuda::get_stream, ::cuda::stream_ref{}); | ||
| auto stream = | ||
| _CUDA_STD_EXEC::__query_or(env, ::cuda::get_stream, ::cuda::stream_ref{static_cast<cudaStream_t>(0)}); |
Contributor
There was a problem hiding this comment.
I am wondering whether we want something that is named as default_stream, rather than always using static_cast<cudaStream_t>(0)
Contributor
Author
There was a problem hiding this comment.
I would say the uglier NULL stream usage looks the better ð
Contributor
ðĻ CI finished in 2d 17h: Pass: 97%/205 | Total: 4d 04h | Avg: 29m 31s | Max: 1h 46m | Hits: 86%/320878
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| CCCL Packaging | |
| +/- | libcu++ |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| +/- | CCCL Packaging |
| +/- | libcu++ |
| +/- | CUB |
| +/- | Thrust |
| +/- | CUDA Experimental |
| +/- | stdpar |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
ðâ Runner counts (total jobs: 205)
| # | Runner |
|---|---|
| 128 | linux-amd64-cpu16 |
| 23 | windows-amd64-cpu16 |
| 14 | linux-amd64-gpu-h100-latest-1 |
| 14 | linux-amd64-gpu-rtxa6000-latest-1 |
| 12 | linux-arm64-cpu16 |
| 11 | linux-amd64-gpu-rtx2080-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
Contributor
ðĻ CI finished in 2h 58m: Pass: 98%/205 | Total: 1d 18h | Avg: 12m 19s | Max: 1h 16m | Hits: 98%/324669
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| CCCL Packaging | |
| +/- | libcu++ |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| +/- | CCCL Packaging |
| +/- | libcu++ |
| +/- | CUB |
| +/- | Thrust |
| +/- | CUDA Experimental |
| +/- | stdpar |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
ðâ Runner counts (total jobs: 205)
| # | Runner |
|---|---|
| 128 | linux-amd64-cpu16 |
| 23 | windows-amd64-cpu16 |
| 14 | linux-amd64-gpu-h100-latest-1 |
| 14 | linux-amd64-gpu-rtxa6000-latest-1 |
| 12 | linux-arm64-cpu16 |
| 11 | linux-amd64-gpu-rtx2080-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
bernhardmgruber
approved these changes
Jul 22, 2025
|
|
||
| // Query relevant properties from the environment | ||
| auto stream = _CUDA_STD_EXEC::__query_or(env, ::cuda::get_stream, ::cuda::stream_ref{}); | ||
| auto stream = _CUDA_STD_EXEC::__query_or(env, ::cuda::get_stream, ::cuda::stream_ref{static_cast<cudaStream_t>(0)}); |
Contributor
There was a problem hiding this comment.
Q: Is this the prefered way to construct the cudaStream_t? IIUC, it's a pointer, so cudaStream_t{nullptr} or even cudaStream_t{} would work. Casting an integral zero to a pointer type is a bit of a type violation to me, but I know that's what NULL does and it's legal C++.
Contributor
Author
There was a problem hiding this comment.
Updated to cudaStream_t{}, I am not sure why I used the static_cast way
Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>
Contributor
ðĻ CI finished in 1h 51m: Pass: 97%/205 | Total: 4d 02h | Avg: 28m 49s | Max: 1h 39m | Hits: 85%/337171
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| CCCL Packaging | |
| +/- | libcu++ |
| +/- | CUB |
| Thrust | |
| +/- | CUDA Experimental |
| stdpar | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| +/- | CCCL Packaging |
| +/- | libcu++ |
| +/- | CUB |
| +/- | Thrust |
| +/- | CUDA Experimental |
| +/- | stdpar |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
ðâ Runner counts (total jobs: 205)
| # | Runner |
|---|---|
| 128 | linux-amd64-cpu16 |
| 23 | windows-amd64-cpu16 |
| 14 | linux-amd64-gpu-h100-latest-1 |
| 14 | linux-amd64-gpu-rtxa6000-latest-1 |
| 12 | linux-arm64-cpu16 |
| 11 | linux-amd64-gpu-rtx2080-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
Contributor
ðĐ CI finished in 3h 09m: Pass: 100%/205 | Total: 4d 02h | Avg: 28m 41s | Max: 1h 39m | Hits: 85%/339409
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| CCCL Packaging | |
| +/- | libcu++ |
| +/- | CUB |
| Thrust | |
| +/- | CUDA Experimental |
| stdpar | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| +/- | CCCL Packaging |
| +/- | libcu++ |
| +/- | CUB |
| +/- | Thrust |
| +/- | CUDA Experimental |
| +/- | stdpar |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
ðâ Runner counts (total jobs: 205)
| # | Runner |
|---|---|
| 128 | linux-amd64-cpu16 |
| 23 | windows-amd64-cpu16 |
| 14 | linux-amd64-gpu-h100-latest-1 |
| 14 | linux-amd64-gpu-rtxa6000-latest-1 |
| 12 | linux-arm64-cpu16 |
| 11 | linux-amd64-gpu-rtx2080-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
bernhardmgruber
approved these changes
Jul 22, 2025
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 22, 2025
âĶast usages (#5310) * Deprecate default stream_ref constructor and fix some few last usages * fix memory_resource test * Update the deprecation message Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com> * Fix tests and use ::cudaStream_t{} * Fix format --------- Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com> (cherry picked from commit 24eebf6)
Contributor
|
Successfully created backport PR for |
pciolkosz
added a commit
that referenced
this pull request
Jul 22, 2025
âĶast usages (#5310) (#5334) * Deprecate default stream_ref constructor and fix some few last usages * fix memory_resource test * Update the deprecation message * Fix tests and use ::cudaStream_t{} * Fix format --------- (cherry picked from commit 24eebf6) Co-authored-by: pciolkosz <pciolkosz@nvidia.com> Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>
6 tasks
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
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.
We don't want users to use NULL stream unless they have to, it's either overly synchronizing or inferior to an explicit stream. Its meaning also depends on what device is set current and we want to push for a model without implicit state.
Currently
stream_refhas an argument-less constructor that wraps NULL stream, we should deprecate it and remove it in the next major version.