-
Notifications
You must be signed in to change notification settings - Fork 204
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
Allow default-initializing Thrust vectors #4183
base: main
Are you sure you want to change the base?
Conversation
thrust/thrust/detail/vector_base.h
Outdated
struct default_init_t | ||
{}; | ||
|
||
constexpr default_init_t default_init; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the range of opinions on this feature, I am also fine with wrapping those two entities in an experimental
namespace.
🟨 CI finished in 2h 55m: Pass: 94%/97 | Total: 2d 18h | Avg: 41m 06s | Max: 1h 53m | Hits: 68%/128171
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
stdpar | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | stdpar |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 97)
# | Runner |
---|---|
68 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-arm64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
dffa87c
to
9e07c71
Compare
🟨 CI finished in 1h 30m: Pass: 98%/97 | Total: 2d 15h | Avg: 39m 06s | Max: 1h 12m | Hits: 69%/134281
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
stdpar | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | stdpar |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 97)
# | Runner |
---|---|
68 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-arm64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
@@ -45,6 +45,11 @@ | |||
|
|||
THRUST_NAMESPACE_BEGIN | |||
|
|||
struct default_init_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With cudax we went with uninit_t
which I find easier for non-expert users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought default initialization was the safer way, since you cannot skip constructors. I am fine with also providing uninit_t
. Would you be fine if I opened a separate PR for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I am a bit hesitant here.
The main reason people want to use this is to avoid calling a kernel for instantiation. This breaking for non-trivial types will be a source of frustration / bugs.
We should rather give them the right tool that "works" everywhere, even if it is a sharp edge
🟩 CI finished in 2d 01h: Pass: 100%/97 | Total: 2d 16h | Avg: 39m 42s | Max: 1h 12m | Hits: 69%/134281
|
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
CUB | |
+/- | Thrust |
CUDA Experimental | |
stdpar | |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
libcu++ | |
+/- | CUB |
+/- | Thrust |
CUDA Experimental | |
+/- | stdpar |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 97)
# | Runner |
---|---|
68 | linux-amd64-cpu16 |
9 | windows-amd64-cpu16 |
6 | linux-arm64-cpu16 |
6 | linux-amd64-gpu-rtxa6000-latest-1 |
3 | linux-amd64-gpu-h100-latest-1 |
3 | linux-amd64-gpu-rtx4090-latest-1 |
2 | linux-amd64-gpu-rtx2080-latest-1 |
I was having this changeset for a while now, but didn't push it, since the discussion in #1992 and among the team was somewhat controversial. However, I keep getting asked about this every now and then, so here is it: an overload for a Thrust vector's constructor and
resize
member function to turn value initialization into default initialization. I refer to #1992 for rational and discussion.Fixes: #1992
Compiling
dv.cu
:with
and dumping the SASS shows that only code for
cub::detail::EmptyKernel
is generated.