Repository navigation
Optimize WarpReduce for large integers (Sum) - #11623
Merged
Merged
Conversation
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/cccl/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughWarp Redux adds SM80 implementations for 64-bit and 128-bit integral plus reductions. The change adds integer word conversion and carry helpers, updates instruction-count documentation, adds wide-integer reduction tests, and supplies an explicit benchmark template argument. ChangesWide-integer warp plus reductions
Priority: ➖ Normal Change: Refactor Merge Risk: 🟡 Moderate · up to The new wide-integer tests need bounded inputs before they can reliably validate the optimized reduction path. Comment ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: NVIDIA/cccl/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e978e7a1-14bc-4925-a6fb-2228957d4df4
📒 Files selected for processing (5)
cub/benchmarks/bench/reduce/warp_reduce_base.cuhcub/cub/detail/interger_utility.cuhcub/cub/warp/specializations/warp_redux.cuhcub/cub/warp/warp_reduce.cuhcub/test/warp/catch2_test_warp_reduce.cu
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
|
||
| c2h::device_vector<T> d_in(input_size); | ||
| c2h::device_vector<T> d_out(output_size); | ||
| c2h::gen(C2H_SEED(10), d_in); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '185,315p' cub/test/warp/catch2_test_warp_reduce.cu
rg -n 'compute_host_reference|TEST_INT128|void gen|gen\\(' cub/test/warp/catch2_test_warp_reduce.cu c2h/include/c2h/vector_generators.hRepository: NVIDIA/cccl
Length of output: 5247
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- generator symbols ---'
rg -n -C 8 'gen\(|numeric_limits|__int128|int128|float|uniform' c2h/include/c2h/vector_generators.h
printf '%s\n' '--- test-int128 symbols ---'
rg -n -C 6 'TEST_INT128|__int128|wide_integer_type_list' cub/test/warp c2h/include c2h/test
printf '%s\n' '--- generator file outline/size ---'
wc -l c2h/include/c2h/vector_generators.h
ast-grep outline c2h/include/c2h/vector_generators.hRepository: NVIDIA/cccl
Length of output: 6830
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- generator symbols ---'
rg -n -C 8 -e 'gen\(' -e 'numeric_limits' -e '__int128' -e 'int128' -e 'float' -e 'uniform' c2h/include/c2h/vector_generators.h
printf '%s\n' '--- test-int128 symbols ---'
rg -n -C 6 -e 'TEST_INT128' -e '__int128' -e 'wide_integer_type_list' cub/test/warp c2h/include c2h/test
printf '%s\n' '--- generator file outline/size ---'
wc -l c2h/include/c2h/vector_generators.h
ast-grep outline c2h/include/c2h/vector_generators.hRepository: NVIDIA/cccl
Length of output: 6830
🏁 Script executed:
sed -n '1,260p' c2h/include/c2h/vector_generators.h; printf '%s\n' '---'; rg -n -C 4 'TEST_INT128|wide_integer_type_list|__int128' cub/test/warp c2h/include c2h/testRepository: NVIDIA/cccl
Length of output: 6944
🌐 Web query:
NVIDIA CCCL c2h/include/c2h/vector_generators.h c2h::gen numeric_limits float __int128
💡 Result:
<source_evidence>
Citations:
- 1: GitHub issue 617 in NVIDIA/cccl (link omitted to avoid creating a cross-reference)
- 2: https://github.com/NVIDIA/cccl/blob/0d9331dc/c2h/include/c2h/test_util_vec.h
- 3: GitHub pull request 5205 in NVIDIA/cccl (link omitted to avoid creating a cross-reference)
- 4: GitHub issue 1227 in NVIDIA/cccl (link omitted to avoid creating a cross-reference)
- 5: GitHub pull request 3413 in NVIDIA/cccl (link omitted to avoid creating a cross-reference)
- 6: GitHub issue 3801 in NVIDIA/cccl (link omitted to avoid creating a cross-reference)
- 7: GitHub pull request 5372 in NVIDIA/cccl (link omitted to avoid creating a cross-reference)
🏁 Script executed:
set -e
printf '%s\n' '--- vector_generators.h relevant definitions ---'
grep -n -E 'gen|numeric_limits|float|double|__int128|uniform' c2h/include/c2h/vector_generators.h | head -120
printf '%s\n' '--- TEST_INT128 definitions and uses ---'
grep -R -n -E 'define[[:space:]]+TEST_INT128|TEST_INT128|__int128' cub/test/warp c2h/include c2h/test | head -160Repository: NVIDIA/cccl
Length of output: 1870
🏁 Script executed:
set -e
printf '%s\n' '--- generator_common definitions ---'
rg -n -C 12 -e 'gen_values_between' -e 'numeric_limits' -e '__int128' -e 'float' -e 'double' -e 'uniform' c2h/include/c2h/generator_common.h c2h/include/c2hRepository: NVIDIA/cccl
Length of output: 41580
🏁 Script executed:
set -e
printf '%s\n' '--- gen_values_between definitions and callers ---'
rg -n -C 20 'gen_values_between' c2h
printf '%s\n' '--- generator detail after mapper definitions ---'
sed -n '100,240p' c2h/include/c2h/detail/generators.cuhRepository: NVIDIA/cccl
Length of output: 14825
important: Bound the generated wide-integer inputs. For __int128_t and __uint128_t, c2h::gen uses full numeric limits and converts them to float. The signed range subtraction overflows to infinity, and the unsigned maximum converts to infinity. The resulting float-to-integer conversion can therefore produce invalid 128-bit inputs before compute_host_reference runs. Pass explicit bounds whose 32-item sum remains representable, and add deterministic carry values within those bounds.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
davebayer
requested changes
Sep 24, 2026
Contributor
There was a problem hiding this comment.
This can be implemented even better. The i64/u64 version currently does 4 parallel reductions, but it can be done only in 3. Similary, the i128/u128 version does 13 parallel reductions, while I was able to reduce it to just 5.
Here are the implementations and comparisons of your vs my:
u64version (i64is identical): https://godbolt.org/z/oKo3h8P6ru128version (i128is identical): https://godbolt.org/z/rr7x7Exb9
Contributor
|
Given the wide SASS impact, we also need a few representative benchmarks. |
Contributor
Author
|
@davebayer implementation leads to 3x for I64/U64 and 14x for I128/U128 over warp shuffle reduction🎉 |
Jacobfaib
reviewed
Sep 29, 2026
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
🔬 CUB benchmark SASS comparisonHow to request a benchmark run
Targets with a SASS change
|
Contributor
🥳 CI Workflow Results🟩 Finished in 3h 07m: Pass: 100%/202 | Total: 10d 07h | Max: 2h 58m | Hits: 26%/947203See results here. AI failure analysis1. cuOpt CUDA test compilation: sccache missing nvcc module_id output · 1 jobExplanation: The tests-enabled cuOpt build failed inside sccache 0.17.0-rapids.4 while packaging nvcc outputs because an expected module-id file was absent; no CUDA compiler diagnostic identifies a CUB source error. The tests-disabled cuOpt build completed beforehand, so the evidence indicates a cache or distributed-compilation failure rather than a demonstrated PR regression. Evidence: Copy this prompt into a coding agentJobs: |
davebayer
approved these changes
Oct 1, 2026
Contributor
The summary is nice. Would be great as well to post the full benchmark results from nvbench next time, I would be curious to get the bigger picture. Thx! |
19 of 27 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.
Description
Our current
WarpReduceimplementation performs 2x5=10 shuffle instructions for 64-bit integers and 4x5=20 for 128-bit integers.We can do better!
The PR maps large integers sum reduction to warp redux operations.
This leads to 2.4x better perf for
I64, and 5.8x forI128.Todo: check if also improvesDeviceReduceDeviceReduce::Sumimproves very slightly 1% forI64and from 1% to 7% forI128