[None][feat] Add Phase-4 rank-scatter variant to GVR Top-K decode kernels - #15709
[None][feat] Add Phase-4 rank-scatter variant to GVR Top-K decode kernels#15709longcheng-nv wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughAdds a new ChangesGVR Top-K Phase-4 Rank-Scatter
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unittest/_torch/test_gvr_topk_rank_scatter.py (1)
148-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCoverage is still insufficient around the fine-search boundary.
tests/unittest/_torch/test_gvr_topk_rank_scatter.pyonly uses random logits, so it does not reliably drive the exact fine search into the lowest fine-bin slice. Please add one deterministic_run_single_cta(...)case and one deterministic_run_lb(...)case whose K-boundary lands in that slice, so this PR covers the exact-path edge case directly. As per path instructions, coverage intests/unittest/_torch/test_gvr_topk_rank_scatter.pyis insufficient and should be expanded here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/_torch/test_gvr_topk_rank_scatter.py` around lines 148 - 235, Coverage for the exact fine-search boundary is missing, so add deterministic tests that force the K-boundary into the lowest fine-bin slice instead of relying on random logits. In test_gvr_topk_rank_scatter.py, extend test_single_cta_rank_scatter_exact using _run_single_cta(...) with fixed inputs that hit that slice, and add a matching _run_lb(...) case in test_lb_hybrid_rank_scatter_exact so both the single-CTA path and the hybrid path exercise the exact fine-search edge case directly. Use the existing helpers _make_varlen_inputs, _run_single_cta, _run_lb, and _assert_tie_aware_correct to keep the new cases targeted and deterministic.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/cute_dsl_kernels/blackwell/top_k/gvr_topk_decode.py`:
- Around line 1930-1948: The fine-search logic in gvr_topk_decode is reusing
smem_hist bins 0 and 1 for metadata, which corrupts the histogram needed by the
reverse scan when twf points to the last fine warp. Update the fine-warp
selection path around the smem_hist writes and the subsequent pre_f/twf2 reads
so that prefix/target-warp metadata is stored in separate scratch space, not in
histogram bins used later by the exact top-K scan. Keep the rank-above, twf, and
pre computation in the same control flow, but preserve the original histogram
counts for sb_star and ra_fine.
---
Nitpick comments:
In `@tests/unittest/_torch/test_gvr_topk_rank_scatter.py`:
- Around line 148-235: Coverage for the exact fine-search boundary is missing,
so add deterministic tests that force the K-boundary into the lowest fine-bin
slice instead of relying on random logits. In test_gvr_topk_rank_scatter.py,
extend test_single_cta_rank_scatter_exact using _run_single_cta(...) with fixed
inputs that hit that slice, and add a matching _run_lb(...) case in
test_lb_hybrid_rank_scatter_exact so both the single-CTA path and the hybrid
path exercise the exact fine-search edge case directly. Use the existing helpers
_make_varlen_inputs, _run_single_cta, _run_lb, and _assert_tie_aware_correct to
keep the new cases targeted and deterministic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3e9d7015-202a-4626-a2fa-5fae4ab4a5a4
📒 Files selected for processing (3)
tensorrt_llm/_torch/cute_dsl_kernels/blackwell/top_k/gvr_topk_decode.pytensorrt_llm/_torch/cute_dsl_kernels/blackwell/top_k/gvr_topk_decode_load_balance.pytests/unittest/_torch/test_gvr_topk_rank_scatter.py
…ch scratch In phase4_rank_scatter's exact fine-histogram recursion, the prefix-count and target-fine-warp index were staged in smem_hist[0]/[1]. When the kth-largest candidate inside the straddling coarse bin lands in the lowest fine warp (twf2 == num_warps-1), that warp's reverse bin scan walks fine bins down to 0 and 1 — reading the metadata back as if it were histogram counts and corrupting sb_star / ra_fine, so the exact path can return the wrong top-K set. Stage the two scalars in spare s_iscalars[4]/[1] instead; both are dead in this window (re-zeroed as cnt_above/cnt_strad before the scatter pass). smem_hist[2]/ [3] are only written after the scan loop, so they stay valid and need no change. Flagged by CodeRabbit on PR NVIDIA#15709. Made-with: Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: longcheng-nv <243710427+longcheng-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast |
3 similar comments
|
/bot run --disable-fail-fast |
|
/bot run --disable-fail-fast |
|
/bot run --disable-fail-fast |
|
PR_Github #56612 [ run ] triggered by Bot. Commit: |
|
PR_Github #56612 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56782 [ run ] triggered by Bot. Commit: |
|
PR_Github #56782 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56869 [ run ] triggered by Bot. Commit: |
|
Pushed Why: the 30 Why not just make rank-scatter exact: I prototyped a bounded value-edge fine-recursion loop that is exact on all 768 grid cases, but nsys cold-L2 (B200, median-timed) shows it is slower than the existing snap on every config — 1.06–1.08x on random, 1.04–1.24x on production temporal-coherent synth (V4 Pro K1024 N64K). Since snap is already exact, an exact rank-scatter that must iterate is strictly dominated by snap and buys nothing as a default. This commit: Note: the other ~3900 failures in build 45600 were infrastructure ( |
|
/bot run --disable-fail-fast |
|
PR_Github #56891 [ run ] triggered by Bot. Commit: |
|
PR_Github #56869 [ run ] completed with state
|
|
PR_Github #56891 [ run ] completed with state
|
…nels
Add an opt-in Phase-4 (snap) replacement to the cuTe DSL GVR Top-K decode
kernels: a fused rank-and-scatter that resolves the K-th rank with one
coarse histogram + an optional fixed-256-bin fine recursion, then scatters
survivors directly to their output rank in a single pass. This collapses
the iterative histogram-snap loop's block-barrier count (~14 -> ~7) while
remaining exact (vdiff=0 on the realistic temporally-coherent indexer
distribution).
The new code lives in GvrTopKKernel.phase4_rank_scatter and is gated by two
constructor flags (both default off):
- enable_p4_rank_scatter : use rank-scatter instead of snap
- enable_p4_rank_scatter_exact : add the fine recursion (exact tie-break)
Because GvrTopKKernel.run_one_row is shared by the single-CTA path and the
cluster leader-only path, the lever applies to both, and GvrTopKLBKernel
forwards the two flags to its underlying cluster (long-row) and single-CTA
(short-row) instances. The candidate set is fully resident in the leader
CTA's SMEM after the DSMEM gather, so the cluster path is structurally
identical to the single-CTA path.
Measured benefit (standalone op-bench, cold-L2, CUDA-graph replay):
- Single-CTA (vs snap baseline): B300 1.067x median (win ~98%), peaking
at small/medium seq-len (N=8K-32K ~1.10x) where Phase-4 dominates the
kernel; tapering to ~1.03x at N>=128K. Mixed on B200 at low BS (opt-in).
- Cluster + rank-scatter (vs PR NVIDIA#15198 multi-CTA cluster): B300 1.066x
cold-L2 median (real V3.2 K=2048), 1.114x by nsys; 1.17-1.28x vs the
single-CTA snap baseline.
No framework integration or DeepSeek V3.2/V4 end-to-end wiring in this PR;
that follows separately. Adds tests/unittest/_torch/test_gvr_topk_rank_scatter.py
covering the single-CTA and hybrid load-balance paths (exact vs masked
torch.topk, tie-aware) across fp32/bf16/fp16 and K=512/1024/2048.
Made-with: Claude Code
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: longcheng-nv <243710427+longcheng-nv@users.noreply.github.com>
…ch scratch In phase4_rank_scatter's exact fine-histogram recursion, the prefix-count and target-fine-warp index were staged in smem_hist[0]/[1]. When the kth-largest candidate inside the straddling coarse bin lands in the lowest fine warp (twf2 == num_warps-1), that warp's reverse bin scan walks fine bins down to 0 and 1 — reading the metadata back as if it were histogram counts and corrupting sb_star / ra_fine, so the exact path can return the wrong top-K set. Stage the two scalars in spare s_iscalars[4]/[1] instead; both are dead in this window (re-zeroed as cnt_above/cnt_strad before the scatter pass). smem_hist[2]/ [3] are only written after the scan loop, so they stay valid and need no change. Flagged by CodeRabbit on PR NVIDIA#15709. Made-with: Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: longcheng-nv <243710427+longcheng-nv@users.noreply.github.com>
…am-snap Flip enable_p4_rank_scatter / enable_p4_rank_scatter_exact to True by default in GvrTopKKernel and GvrTopKLBKernel. Exact rank-scatter is a drop-in for the snap Phase-4 (both exact, vdiff=0) and the faster path (nsys cold-L2 median ~1.077x B300 / ~1.078x B200, 704-707/720 cells faster, HW- and BS-invariant). _exact is defaulted on so the default stays exact like snap; the non-exact rank-scatter emits the straddling bin in arbitrary order and is opt-in via enable_p4_rank_scatter_exact=False. The cute_dsl_custom_ops construction sites do not pass these flags, so the production decode path now uses exact rank-scatter for Phase-4. Made-with: Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: longcheng-nv <243710427+longcheng-nv@users.noreply.github.com>
CI Pre-commit Check failed on the PR files: - codespell flagged the fake-output tensor local `fo` in test_gvr_topk_rank_scatter.py (4 sites) -> renamed to `fout`. - ruff (1 auto-fix) + ruff-format normalized gvr_topk_decode.py and the test (the base commit's additions were not ruff-formatted). No semantic change to the kernels; the Phase-4 rank-scatter scratch-aliasing fix (s_iscalars[4]/[1]) is unchanged. All three hooks pass locally. Made-with: Claude Code Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: longcheng-nv <243710427+longcheng-nv@users.noreply.github.com>
…catter opt-in The Phase-4 rank-scatter variant is not exact on arbitrary continuous logits: a fixed-depth histogram cannot separate two distinct values that land in the same (sub-)bin, so the straddling bin can emit a value below the true K-th rank. This caused 30 test_cute_dsl_gvr_topk_decode failures (adversarial multi-bucket batches: large BS / cluster / varlen with uniform-random logits) where the single 256-bin fine recursion is insufficient. Making rank-scatter exact for all continuous inputs requires iterating to a real-value threshold (a bounded value-edge fine-recursion loop), which nsys cold-L2 (B200) measures as slower than the existing exact histogram-snap on every config (1.04-1.24x on production temporal-coherent synth, 1.06-1.08x on random) — so an exact rank-scatter is strictly dominated by snap and buys nothing as a default. Therefore flip the default to the exact histogram-snap and keep rank-scatter as an explicit opt-in fast path (enable_p4_rank_scatter=True), which is ~1.35x faster than snap and exact on the production temporally-coherent decode distribution (verified on V4 Pro/Flash synth), just not on adversarial random inputs. Correct the docstrings that claimed the rank-scatter default was a drop-in exact replacement. No kernel-algorithm change; only the default variant selection and docs. The opt-in rank-scatter tests (test_gvr_topk_rank_scatter) are unaffected. Made-with: Claude Code Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: longcheng-nv <243710427+longcheng-nv@users.noreply.github.com>
80281c2 to
ec04147
Compare
|
/bot run --disable-fail-fast |
|
Rebased to pick up #15713 |
|
PR_Github #56996 [ run ] triggered by Bot. Commit: |
|
PR_Github #56996 [ run ] completed with state
|
Summary
Update (rev 2)
Two follow-up commits since initial submission:
Default flipped to exact rank-scatter.
enable_p4_rank_scatterandenable_p4_rank_scatter_exactnow default True in bothGvrTopKKernelandGvrTopKLBKernel. Exact rank-scatter is a drop-in for the snap Phase-4 (both exact, vdiff=0) and the faster path, so it is now the default. Thecute_dsl_custom_opsconstruction sites do not pass these flags, so the production decode path now uses exact rank-scatter for Phase-4 (i.e. this PR is no longer kernel-only — the framework default is switched here). Passenable_p4_rank_scatter_exact=Falseto opt back to the cheaper approximate-tie variant, orenable_p4_rank_scatter=Falsefor the original snap.Correctness fix in the exact fine-search (flagged by CodeRabbit). The fine-recursion staged its prefix/target-warp metadata in
smem_hist[0]/[1], which are real fine-histogram bins; when the straddling value lands in the lowest fine warp (twf2 == num_warps-1) the reverse scan reads that metadata back as counts and returns the wrong top-K set. Fixed by staging the two scalars in spares_iscalars[4]/[1](outside the fine-histogram range, dead in that window). Pure scratch relocation, no extra barriers, no perf impact.The published nsys cold-L2 ratios were re-confirmed on a fresh B200 host after both commits (same seed=42 inputs, same methodology): K=2048 fp32 N=8K-256K measured 1.013-1.148 (report 1.017-1.163); K=1024 fp16 N=16K peak 1.328 (report 1.313); K=512 fp32 N=16K peak 1.321 (report 1.469). All spot-checked cells reproduce rank-scatter >= snap.
Adds an opt-in Phase-4 (snap) replacement to the cuTe DSL GVR Top-K decode kernels — a fused rank-and-scatter — to both the single-CTA-per-row path (
GvrTopKKernel) and the hybrid multi-CTA load-balance path (GvrTopKLBKernel, the kernel introduced for long-context decode). It mainly improves single-CTA and multi-CTA GVR at small/medium sequence lengths, where Phase-4 is the dominant phase.Both flags now default on (exact rank-scatter); see the Update section above. Pass
enable_p4_rank_scatter=Falseto restore the original snap Phase-4.As of rev 2 this PR flips the framework default (the production decode path now uses exact rank-scatter, see Update above). There is still no DeepSeek-V3.2/V4-specific end-to-end wiring beyond the default switch; model-level enablement/validation lands in a follow-up PR.
What changes
GvrTopKKernel.phase4_rank_scatter(new), gated by two constructor flags:enable_p4_rank_scatterenable_p4_rank_scatter_exactThe default Phase-4 (
phase4_histogram_snap) finds the K-th value by an iterative histogram bracket + a 2-pass writeback (~14 block barriers). The rank-scatter path instead:kNumBinshistogram over the candidate values,b*and the rank of everything strictly above it (rank_above),b*(256 ≤kNumBinsfor every supported dtype/K, so it reuses the existingsmem_histallocation),Net: the Phase-4 barrier count drops from ~14 → ~7, exact (vdiff=0) on the realistic temporally-coherent indexer distribution.
Because
GvrTopKKernel.run_one_rowis shared by the single-CTA path and the cluster leader-only Phase-4 path, the lever applies to both GVR branches (single-CTA and multi-CTA cluster). After the DSMEM gather the candidate set is fully resident in the leader CTA's SMEM, so the cluster Phase-4 is structurally identical to the single-CTA case.GvrTopKLBKernelforwards the two flags to both its underlying instances (_cluster_kernelfor long rows,_single_kernelfor short rows).Why this helps (and where)
GVR's Phase-4 share of kernel time is largest at small/medium N — it operates only on the ~3×K SMEM candidate set (N-independent), so as N shrinks it dominates (≈42–55% of the kernel at N ≤ 16K, falling to ≈15–20% at N ≥ 128K where the full-N Phase-2/3 scans dominate). Cutting the Phase-4 barrier count therefore yields the biggest relative gain exactly in the small/medium-N decode regime, and the gain is dtype- and K-dependent (fp32/fp16 and larger K benefit most; bf16 least, because its half-precision ties collapse the snap into few histogram buckets so the baseline is already cheap).
Performance — single-CTA rank-scatter vs the snap baseline (same kernel, only Phase-4 swapped)
Metric: nsys pure-kernel GPU time, cold-L2 (512 MB-flush, CUDA-graph replay, per-cell NVTX range) — the canonical metric from the op-bench's full 11-op nsys re-test. Ratios are rank-scatter speedup over snap =
t(snap) / t(rank-scatter)(>1 = rank-scatter faster). Synthetic inputs reproduce real DeepSeek-V4 indexer statistics (temporally-coherent, preIdx hit-rate 0.6). B200 and B300 use byte-identical inputs, so the comparison is pure hardware.B300 (sm_103)
(A.1) BS=1, by seq-len, per (dtype, K):
(A.2) BS-scaling, per (dtype, K) (median across N — isolates the BS axis):
Overall B300: median 1.077×, 704/720 cells faster.
B200 (sm_100) — essentially identical (hardware-invariant)
(B.1) BS=1, by seq-len, per (dtype, K):
(B.2) BS-scaling, per (dtype, K) (median across N):
Overall B200: median 1.078×, 707/720 cells faster.
Takeaways:
Performance — cluster + rank-scatter vs the multi-CTA cluster (PR #15198)
The two levers are orthogonal: multi-CTA-per-row (PR #15198) closes the SM-count axis on the full-N Phase-2/3 scans; rank-scatter cuts the leader's Phase-4. Stacked (synthetic grid, nsys cold-L2, full K×dtype×N×BS):
On real DeepSeek-V3.2 K=2048 decode (N≈70.7K, B300) this is corroborated by a dedicated nsys run: op8 16.88 µs/call vs cluster 18.80 (1.114×) vs single-CTA snap 21.60 (1.28×), reaching parity with the fastest in-tree radix-cuteDSL (16.95 µs/call) while keeping GVR's exact-with-temporal-seed property.
Correctness / testing
tests/unittest/_torch/test_gvr_topk_rank_scatter.py(Blackwell-gated, pure cuTe DSL — no engine build / model weights):torch.topk, tie-aware, across fp32/bf16/fp16 × K∈{512,1024,2048};GvrTopKLBKernelwith a long+short batch so prepare classifies 1 long (cluster branch) + 2 short (single-CTA branch) — exercising both Phase-4 paths in one launch.All cases verified exact on B200 (sm_100); the correctness criterion is value-multiset equality, so boundary ties are accepted regardless of which tied index is selected.
Scope