Repository navigation
Add dynamic CUB dispatch for unique_by_key - #3816
Conversation
|
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. |
|
/ok to test |
|
/ok to test |
🟨 CI finished in 1h 36m: Pass: 94%/93 | Total: 2d 15h | Avg: 40m 50s | Max: 1h 14m | Hits: 65%/127608
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
…because this allows us to define it such that we can use the ActivePolicyT template parameter of invoke() as its policy, rather than the policy hub's MaxPolicy. ActivePolicyT will be the correct policy for the current ptx version, since calling invoke() from dispatch() causes us to move down the chained policies until we get to the correct one.
🟩 CI finished in 1h 37m: Pass: 100%/93 | Total: 2d 14h | Avg: 40m 04s | Max: 1h 21m | Hits: 74%/133653
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
… needed because this allows us to define it such that we can use the ActivePolicyT template parameter of invoke() as its policy, rather than the policy hub's MaxPolicy. ActivePolicyT will be the correct policy for the current ptx version, since calling invoke() from dispatch() causes us to move down the chained policies until we get to the correct one." This reverts commit 2a17133.
…s it from the c.parallel layer. Also we don't need it to have any run-time state
| typename VSMemHelperPolicyT = detail::vsmem_helper_default_fallback_policy_t< | ||
| typename PolicyHub::MaxPolicy::UniqueByKeyPolicyT, |
There was a problem hiding this comment.
I think this is a bug, we must pass the ActivePolicy::UniqueByKeyPolicyT selected by Invoke here.
There was a problem hiding this comment.
Here is my rationale for this change. Originally VSMemHelperT was instantiated in Invoke() in order to pass in the ActivePolicyT::UniqueByKeyPolicyT. We then got BLOCK_THREADS and ITEMS_PER_THREAD by doing:
constexpr auto block_threads = VsmemHelperT::agent_policy_t::BLOCK_THREADS;
constexpr auto items_per_thread = VsmemHelperT::agent_policy_t::ITEMS_PER_THREAD;However, in order to enable dynamic dispatch, we need to use the run-time values from ActivePolicy, so we added an ActivePolicy argument to Invoke() which we will pass in from the c.parallel side. No we have to use this argument to retrieve block_threads and items_per_thread instead of VsmemHelperT::agent_policy_t, so that it looks something like this:
template<typename ActivePolicyT>
void Invoke(ActivePolicyT policy) {
// ...
const auto block_threads = VsmemHelperT::BlockThreads<ActivePolicyT>(policy);
const auto items_per_thread = VsmemHelperT::ItemsPerThread<ActivePolicyT>(policy);
// ...
}
template <...>
struct vsmem_helper {
// BlockThreads definition (similarly for ItemsPerThread)
template <typename AgentPolicyT>
_CCCL_HOST_DEVICE static constexpr int BlockThreads(AgentPolicyT /* policy */)
{
return AgentPolicyT::BLOCK_THREADS;
}
};So I added the BlockThreads() and ItemsPerThread() methods to the vsmem_helper class in this PR (one for the normal case and one for fallback, see changes to util_vsmem.h). They will use the ActivePolicy from Invoke(), and not the policy vsmem_helper was instantiated with. So it doesn't really matter that we are not instantiating vsmem_helper with the active policy from Invoke().
Now the question is, why do we need to have VSMemHelperT be a template parameter of the dispatch class and not Invoke() directly? The reason is that we do not call Invoke() from c.parallel, we only instantiate the dispatch class and call Dispatch(), and we need to provide an implementation on the c.parallel side which will implement BlockThreads() and call Dispatch() like this:
struct c_parallel_vsmem_helper {
int BlockThreads(c_runtime_tuning_policy policy) {
return policy.BlockThreads();
}
};
Dispatch<..., c_parallel_vsmem_helper, ...>::Dispatch(...);
🟩 CI finished in 1h 33m: Pass: 100%/93 | Total: 2d 13h | Avg: 39m 33s | Max: 1h 20m | Hits: 74%/133653
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
🟩 CI finished in 1h 36m: Pass: 100%/93 | Total: 2d 13h | Avg: 39m 34s | Max: 1h 18m | Hits: 74%/133653
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
|
/ok to test |
🟨 CI finished in 2h 58m: Pass: 86%/93 | Total: 2d 07h | Avg: 36m 03s | Max: 1h 55m | Hits: 75%/115246
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
…und an nvcc 12.0 template instantiation issue with vsmem_helper_fallback_policy_t
|
/ok to test |
🟨 CI finished in 1h 32m: Pass: 96%/93 | Total: 2d 12h | Avg: 39m 17s | Max: 1h 17m | Hits: 74%/130026
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
|
/ok to test |
|
/ok to test |
🟩 CI finished in 1h 31m: Pass: 100%/93 | Total: 2d 15h | Avg: 40m 44s | Max: 1h 17m | Hits: 62%/133929
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
| return VSMemHelperDefaultFallbackPolicyT<ActivePolicyT, Ts...>::vsmem_per_block; | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Ideally VSMemHelper would be templated on typename... Ts to avoid repetition, but for some reason nvcc 12.0 does not like that, so I resorted to making it a template parameter of all the methods, which is redundant and looks a bit uglier, but works.
🟩 CI finished in 1h 29m: Pass: 100%/93 | Total: 2d 13h | Avg: 39m 49s | Max: 1h 23m | Hits: 75%/133929
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
|
I checked the sass diff following my recent changes and there were no changes |
|
I re-started reviewing this PR 2 times already and could not reach confidence in my understanding of the motivation behind those changes. I think I lack some understanding of how the Python side is going to invoke the CUB kernels. I would like somebody else to review and approve this PR. I need to do some more research on the Python side, but I don't have bandwidth for this now. |
🟩 CI finished in 1h 48m: Pass: 100%/93 | Total: 2d 12h | Avg: 39m 09s | Max: 1h 21m | Hits: 75%/133929
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
shwina
left a comment
There was a problem hiding this comment.
LGTM, would just ask for some feedback from the C++ folks on the SFINAE bit
🟩 CI finished in 1h 17m: Pass: 100%/93 | Total: 22h 06m | Avg: 14m 15s | Max: 1h 15m | Hits: 94%/133929
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
🟩 CI finished in 1h 35m: Pass: 100%/93 | Total: 2d 11h | Avg: 38m 21s | Max: 1h 14m | Hits: 76%/133929
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| Thrust | |
| CUDA Experimental | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| +/- | CUB |
| +/- | Thrust |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 93)
| # | Runner |
|---|---|
| 66 | linux-amd64-cpu16 |
| 9 | windows-amd64-cpu16 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 4 | linux-arm64-cpu16 |
| 3 | linux-amd64-gpu-h100-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
* Make vsmem_helper a template on the dispatch class so that we can pass it from the c.parallel layer. Also we don't need it to have any run-time state * Pass VSMemHelper as template parameter to unique by key kernel * Move the template parameters of VSMemHelper to its methods to workaround an nvcc 12.0 template instantiation issue with vsmem_helper_fallback_policy_t * Make KeyT and ValueT templates for DispatchUniqueByKey
Description
Closes #3806
No SASS difference for
cub.cpp17.test.device_select_unique_by_key.lid_0Checklist