Port thrust extrema algs to use CUB - #4970
Conversation
| // TODO(bgruber): the previous thrust implementation avoided creating an initial value. Should we bring this back? | ||
| // There is no reduction in CUB without init. | ||
| const auto offset = thrust::cuda_cub::detail::reduce_n_impl( | ||
| policy, | ||
| zip_first, | ||
| num_items, | ||
| tuple_t{cub::FutureValue(first), offset_t{0}}, |
There was a problem hiding this comment.
I picked the first element of the input range as initial value, since I cannot come up with an identity element for any user-defined binary predicate. I am not worried about the performance impact of loading that first element twice. However, it still feels a bit hackish. Does anyone have any better ideas?
| [](execution_policy<Derived>& policy, const tuple_t* result_ptr) { | ||
| // TODO(bgruber): I only want to download the offset (element 1) and not the value (element 0), but how can I | ||
| // legally form a pointer to that tuple element? | ||
| // return get_value(policy, &thrust::get<1>(*result_ptr)); | ||
| return thrust::get<1>(get_value(policy, result_ptr)); |
There was a problem hiding this comment.
@miscco If I have a tuple<A, B>* pointing to device memory, can I legally get a B* to the second tuple element? get<1> requires to dereference the pointer, which ... may be UB?
There was a problem hiding this comment.
I am wondering about the situation here.
If there is a tuple<A, B> at the memory location the pointer points at, then it is perfectly fine to use get even if the element in the A slot is not properly initialized
|
Thrust's version previously supported large custom types using virtual shared memory. That's currently not supported by |
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
6517eed to
a7a5113
Compare
🟨 CI finished in 1h 50m: Pass: 63%/130 | Total: 2d 14h | Avg: 29m 03s | Max: 1h 16m | Hits: 78%/73181
|
| 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: 130)
| # | Runner |
|---|---|
| 89 | linux-amd64-cpu16 |
| 11 | windows-amd64-cpu16 |
| 10 | linux-arm64-cpu16 |
| 7 | linux-amd64-gpu-rtx2080-latest-1 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
Yes. Running out of SMEM is extremely unlikely so we just provide a static assert to users instead, see #6062. |
a7a5113 to
bc3eb5d
Compare
a294b03 to
23414ee
Compare
😬 CI Workflow Results🟥 Finished in 4h 38m: Pass: 97%/159 | Total: 6d 13h | Max: 4h 24m | Hits: 74%/189478See results here. |
23414ee to
48452f3
Compare
|
I found some problems with the current approach. It's somewhat of a hack anyway. I think to properly implement this, we would need some extensions to |
48452f3 to
42f6456
Compare
|
Superseded by #8292 |
Fixes #1626
thrust.bench.reduce.basic.baseon RTX 5090