Repository navigation
[CUB] Vectorize DeviceReduce loads for any contiguous iterator - #12326
Merged
bernhardmgruber merged 1 commit intoOct 31, 2026
Merged
Conversation
Contributor
|
Important Review skippedThe saved review base belongs to an older reviewed commit. This saved history cannot establish the base for an incremental review. Comment You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 Summary
Change: Feature Merge Risk:
 |
This comment has been minimized.
This comment has been minimized.
miscco
approved these changes
Oct 29, 2026
miscco
left a comment
miscco
left a comment
Contributor
There was a problem hiding this comment.
This looks good to me, it also fixed the dropping of the cache modifier in the fallback path which is nice :)
miscco
reviewed
Oct 29, 2026
Comment on lines
+221
to
+223
| && THRUST_NS_QUALIFIER::is_contiguous_iterator_v<InputIteratorT> | ||
| // TODO(bgruber): remove the check for is_primitive<ValueT> in CCCL 4.0 | ||
| && (is_primitive<InputT>::value || ::cuda::is_trivially_copyable_v<InputT>) |
Contributor
There was a problem hiding this comment.
Forgot to send: This is still allowing vectorization for volatile base types.
Should we check for that?
Contributor
Author
There was a problem hiding this comment.
If volatile is emitted via the cache modifier, then this should be retained by the rewarp of the vectorized pointer. Otherwise, we drop it I believe, which is what the old code did as well.
Edit: Claude confirms
This comment has been minimized.
This comment has been minimized.
bernhardmgruber
enabled auto-merge (squash)
October 29, 2026 20:57
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
SASS diff analysis I downloaded the SASS diff artifacts and went through them. Segmented top-k (4 targets): there are no SASS changes in any top-k kernel. The only changed kernels in those targets are Reductions (
Register counts, spills and occupancy can't be read from the diff artifacts, so a benchmark run is needed to judge the performance impact. Analysis done by Claude. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔇 Additional comments: 1
ci/bench.yaml-48-48: 1
48-48:Major · Stability & Availability · Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.important: Do not enable
gb300with the current benchmark runner mapping.The workflow hard-codes
linux-amd64-gpu-${{ inputs.gpu }}-latest-1, so this entry selectslinux-amd64-gpu-gb300-latest-1. The repository's GB300 lanes usearm64, and the runner-label template includes the CPU architecture. This benchmark job will target the wrong runner instead of the GB300 pool. Carry the CPU architecture through benchmark dispatch, or removegb300until the workflow supports the arm64 label. (github.com)As per path instructions, fix the matrix-to-runner mapping before enabling this GPU.
Source: Path instructions
-
-
- Source mode: Auto
- Repository: NVIDIA/cccl
- Branch: main
- File: .coderabbit.yaml
- Commit: 5939983
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
8ba92aef-6f3c-40d9-b35d-0c3624823a43
-
ci/bench.yaml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
-
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
NVBench robust-intervals outcomes from bench run 40966440359 (base:
No regressions (SLOW) on any GPU. 🤖 Generated with Claude Code |
Contributor
Author
|
Archived robust compare-intervals output of h100l4t4rtxa6000rtxpro6000🤖 Generated with Claude Code |
bernhardmgruber
force-pushed
the
reduce-contiguous-vectorization
branch
from
October 30, 2026 20:59
619c8a4 to
43fc27c
Compare
Unwrap contiguous iterators in AgentReduce instead of requiring raw pointers, and apply the cache modifier on top of the unwrapped pointer. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
bernhardmgruber
force-pushed
the
reduce-contiguous-vectorization
branch
from
October 30, 2026 21:00
43fc27c to
4de9b4d
Compare
Jacobfaib
approved these changes
Oct 30, 2026
Contributor
|
/sass diff ok 4de9b4d |
Contributor
|
Error: @Jacobfaib, the SASS approval job is not ready. Wait for the "Require SASS approval" job to finish, then post |
Contributor
🔬 Benchmark SASS comparisonHow to request a benchmark run
Targets with a SASS change
|
Contributor
|
/sass diff ok 4de9b4d |
Contributor
|
Error: @Jacobfaib, no visible matching SASS report precedes your approval command. Review the current SASS report, make sure it is visible, then post |
Contributor
|
/sass diff ok 4de9b4d |
Contributor
|
@Jacobfaib, your SASS approval is recorded, but GitHub rejects CI rerun requests while it is running. After the current CI run finishes, rerun its failed jobs to apply the approval. |
This comment has been minimized.
This comment has been minimized.
Contributor
🥳 CI Workflow Results🟩 Finished in 3h 21m: Pass: 100%/214 | Total: 9d 18h | Max: 3h 21m | Hits: 34%/820730See results here. |
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.
AgentReduceonly vectorized loads for raw pointers. It now unwraps any contiguous iterator (e.g.thrust::device_ptr,device_vectoriterators) and applies the cache modifier on top of the resulting pointer, resolving the TODO inagent_reduce.cuh.🤖 Generated with Claude Code