Skip to content

fix(dsv4): avoid fused TopK v2 on Hopper - #33835

Closed
Emilio597 wants to merge 1 commit into
sgl-project:mainfrom
Emilio597:fix/dsv4-topk-v2-hopper
Closed

fix(dsv4): avoid fused TopK v2 on Hopper#33835
Emilio597 wants to merge 1 commit into
sgl-project:mainfrom
Emilio597:fix/dsv4-topk-v2-hopper

Conversation

@Emilio597

@Emilio597 Emilio597 commented Aug 6, 2026

Copy link
Copy Markdown

Motivation

DeepSeek-V4 decode can produce incomplete DSA TopK v2 output on Hopper when a small mixed batch crosses the 32K C4 cluster floor. In an 8x H200 TP8 deployment, this surfaced as a reproducible downstream CUDA illegal-memory access when one long request crossed raw token 131072 while seven short requests remained active.

The failure is in the current fused small-batch DSMEM path, not in Marlin, Mooncake, FP8 KV cache, or the general persistent-cluster implementation.

Root cause

Calling topk_transform_512_v2 directly on H200 with an identity page table gave the following result:

C4 seq_len:       32767  32768  32769  33000  2000  12000  16385  1000
unique/512 slots:   512    512    115    207    512    512    512   512

For batch_size <= 15, the small-batch cluster floor is 32768. The two rows just above that floor enter Cluster::forward in topk_small_batch_kernel and return incomplete output on SM90. Register and streaming rows in the same launch are correct.

Additional isolation:

  • Moving the PDL wait before the first seq_lens read did not repair the two failing rows.
  • The existing persistent-cluster path was correct: batch=31, seq_len=65537 returned 512 unique valid indices for all 31 rows.
  • Disabling TopK v2 made the full workload stable, and restoring the original v2 dispatch reproduced the crash at the same C4 boundary.

The direct kernel corruption is deterministic. The link from stale/duplicate TopK slots to the downstream sparse-attention illegal access is supported by the full-model boundary A/B.

Changes

  • Keep the fused small-batch path on SM100+.
  • On Hopper, route the same shapes through the existing persistent-cluster plus main-kernel path.
  • Add a regression test with mixed row lengths immediately below, at, and above the 32768 small-batch cluster floor.

Blackwell dispatch is unchanged.

Validation

Tested environment:

GPU: 8x NVIDIA H200 (SM90a)
Model: DeepSeek-V4-Flash-0731
SGLang base: 7adf2f4a9a4389d1c021a2caf128cf7e2f4adb35
PyTorch: 2.11.0
CUDA toolkit: 13.1
TP: 8
KV cache: FP8 E4M3
MoE runner: Marlin
Context length: 400000
Decode CUDA Graph: enabled

Results with the Hopper fallback:

  • Direct mixed-boundary kernel check: all 8 rows returned 512 unique indices, all in range.
  • Deterministic boundary reproducer (1 x 130800 -> 10000 plus 7 x 2000 -> 10000): all requests completed; 80,000 output tokens; no CUDA error.
  • Full stability gate (40 x 200000 -> 40000, concurrency 40): 40/40 requests completed; 8,000,000 input and 1,600,000 output tokens; 1566.01 output tok/s; HTTP health remained 200.
  • Persistent-path regression (1 x 298933 -> 10000 plus 7 x 9731 -> 10000): all requests completed, including the C4 > 65536 row.
  • The same full stability gate with Mooncake HiCache write-through: 40/40 completed; actual Mooncake SSD read/write counters increased; no CUDA error.

The deployed validation branch also contained a PDL-wait placement cleanup inside the fused kernel. A PDL-only A/B did not fix the corruption, and the fused kernel is not launched on SM90 after this dispatch change. This PR intentionally excludes that unrelated cleanup and contains only the minimal Hopper fallback.

The tested topk_v2.cuh and regression-test base blobs are unchanged on current main as of 2026-08-06.

Related work

Checklist

  • The change is scoped to the affected architecture and dispatch path.
  • A focused regression test is included.
  • git diff --check passes.
  • Full-model correctness and long-running serving validation are reported above.
  • CI (this PR).

CI States

Latest PR Test (Base): ❌ Run #31081928822
Latest PR Test (Extra): ❌ Run #31081928039

DarkSharpness pushed a commit to DarkSharpness/sglang that referenced this pull request Aug 9, 2026
…3.1+

`TopKCluster::forward` selected its phase-3 scatter destination up front:

    const auto cur_out = is_primary ? problem.out : smem->tmp_out;

`problem.out` can be a `shared::cluster` (DSMEM) alias of the elected rank's
buffer, while `tmp_out` is `shared::cta`. Merging both into one pointer variable
makes cicc 13.1+ mis-lower the block-local arm for sm_90a and silently drop
every non-primary rank's staged output: `tmp_out` stays zero, and phase 3.5 then
faithfully copies zeros to perfectly correct DSMEM addresses.

The result is a top-k row where only the primary's slots and the `handle_tie`
tail hold valid indices. Slot-diffing one row against a CUDA 12.9 build shows
exactly one contiguous bad run -- [61, 443] -- with slots 0-60 (the primary's own
scatter) and 444-511 (handle_tie) correct. Downstream sparse attention then
dereferences the garbage slots, which is the illegal memory access reported in
sgl-project#33835.

Fix: keep the two destinations in separate code paths so neither pointer ever
carries two address spaces, and note it so nobody merges them back.

Scope: affects any fused small-batch cluster shape (`batch <= 30` and
`seq_len > cluster_floor`), not just rows near the 32K small-batch floor -- the
floor only makes the path reachable at `batch <= 15`. The persistent-pool path
was never affected because it stages output in global memory.

Toolchain matrix (H200, sm_90a, 157-row suite over the report's shapes plus a
boundary sweep and register/streaming/persistent controls):

    nvcc     before        after
    12.9     0 bad         0 bad
    13.0     0 bad         0 bad
    13.1     157 bad       0 bad
    13.2     157 bad       0 bad
    13.3     157 bad       0 bad

CI builds cu130, which is a clean cell -- that is why this stayed hidden and why
the report came from a CUDA 13.1 deployment.

Also drops the `peer_problem` copy from sgl-project#32910 and states the
block-local pointer at the read-back site instead. That `__builtin_assume` is
load-bearing: removing it reproduces the sgl-project#32830 cicc segfault on 13.1/13.2/13.3.

Validation: 0 bad rows on 12.9/13.0/13.1/13.2/13.3; 1500-iteration randomized
stress and 500 CUDA-graph replays clean; builds for sm_90a and sm_100a on every
toolchain. No performance regression -- worst case +0.1% (noise), and the fused
cluster shapes get 1.3-2.9% faster. sm_100a is compile-verified only; no
Blackwell was available to run on.
@DarkSharpness

Copy link
Copy Markdown
Collaborator

Generated by Claude.

B200 (sm_100a) runtime result: the corruption does not reproduce on Blackwell, so keeping the fused path enabled on SM100 — as this PR does — does not ship a silently-wrong Blackwell. This also closes the "sm_100a is compile-verified only" gap left open in #34167.

Since the trigger is the CUDA toolkit that builds the JIT module rather than the GPU (see #34167), every affected nvcc minor was measured separately, each with a fresh TVM_FFI_CACHE_DIR. Hardware: B200 (sm_100a), single GPU, torch 2.13.0+cu130. Base: unfixed main @ 71043b9.

build nvcc 11 fused/ctrl shapes randomized stress CUDA-graph replays result
main (unfixed) 13.1.115 0 bad 0/100 0/100 clean
main (unfixed) 13.2.78 0 bad 0/300 0/200 clean
main (unfixed) 13.3.73 0 bad 0/100 0/100 clean
#34167 13.2.78 0 bad 0/300 0/200 clean

All three toolkits are cells that are 157/157 bad on H200, so this is an architecture split, not an out-of-range toolchain.

Three checks so this negative is not a false negative:

  1. Right compiler — the generated build.ninja shows the intended nvcc and -gencode=arch=compute_100a,code=sm_100a.
  2. Unfixed sourcetopk_impl.cuh:779 still carries const auto cur_out = is_primary ? problem.out : smem->tmp_out;.
  3. The affected kernel really launched — profiling each case shows all 9 fused shapes dispatching to topk_small_batch_kernel<true>, with the two controls landing in topk_main_kernel<true, 2> (streaming) and topk_persistent_cluster_kernel + topk_main_kernel<true, 3> (persistent pool), as intended. Shapes covered include the reported 1 x 130800 + 7 x 2000, the 32K boundary mix, 8 x 32769, 15 x 49152, 1 x 65537, 8 x 98304, 30 x 131072, and k in {512, 1024, 2048}.

Consistent with codegen: compiling the same unfixed source with one nvcc (13.2) for both targets, topk_small_batch_kernel lowers differently — sm_90a keeps the merged pointer entirely in block-local shared stores (1 generic LD / 44 generic ST / 234 STS), while sm_100a uses generic LD/ST (31 / 65 / 276), and generic addressing is correct for both the DSMEM arm and the CTA-local arm. Suggestive rather than conclusive, but it matches an sm_90a-only mis-lowering.

test/registered/kernels/ops/attention/test_topk_v2.py also passes on B200 with #34167 applied (244 passed).

DarkSharpness pushed a commit to DarkSharpness/sglang that referenced this pull request Aug 9, 2026
…3.1+

`TopKCluster::forward` selected its phase-3 scatter destination up front:

    const auto cur_out = is_primary ? problem.out : smem->tmp_out;

`problem.out` can be a `shared::cluster` (DSMEM) alias of the elected rank's
buffer, while `tmp_out` is `shared::cta`. Merging both into one pointer variable
makes cicc 13.1+ mis-lower the block-local arm for sm_90a and silently drop
every non-primary rank's staged output: `tmp_out` stays zero, and phase 3.5 then
faithfully copies zeros to perfectly correct DSMEM addresses.

The result is a top-k row where only the primary's slots and the `handle_tie`
tail hold valid indices. Slot-diffing one row against a CUDA 12.9 build shows
exactly one contiguous bad run -- [61, 443] -- with slots 0-60 (the primary's own
scatter) and 444-511 (handle_tie) correct. Downstream sparse attention then
dereferences the garbage slots, which is the illegal memory access reported in
sgl-project#33835.

Fix: keep the two destinations in separate code paths so neither pointer ever
carries two address spaces, and note it so nobody merges them back.

Scope: affects any fused small-batch cluster shape (`batch <= 30` and
`seq_len > cluster_floor`), not just rows near the 32K small-batch floor -- the
floor only makes the path reachable at `batch <= 15`. The persistent-pool path
was never affected because it stages output in global memory.

Toolchain matrix (H200, sm_90a, 157-row suite over the report's shapes plus a
boundary sweep and register/streaming/persistent controls):

    nvcc     before        after
    12.9     0 bad         0 bad
    13.0     0 bad         0 bad
    13.1     157 bad       0 bad
    13.2     157 bad       0 bad
    13.3     157 bad       0 bad

CI builds cu130, which is a clean cell -- that is why this stayed hidden and why
the report came from a CUDA 13.1 deployment.

Also drops the `peer_problem` copy from sgl-project#32910 and states the
block-local pointer at the read-back site instead. That `__builtin_assume` is
load-bearing: removing it reproduces the sgl-project#32830 cicc segfault on 13.1/13.2/13.3.

Validation: 0 bad rows on 12.9/13.0/13.1/13.2/13.3; 1500-iteration randomized
stress and 500 CUDA-graph replays clean; builds for sm_90a and sm_100a on every
toolchain. No performance regression -- worst case +0.1% (noise), and the fused
cluster shapes get 1.3-2.9% faster. sm_100a is compile-verified only; no
Blackwell was available to run on.
BBuf pushed a commit that referenced this pull request Aug 10, 2026
…root cause for #33835) (#34167)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Xia-Weiwen pushed a commit to Xia-Weiwen/sglang that referenced this pull request Aug 10, 2026
…root cause for sgl-project#33835) (sgl-project#34167)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Leoyzen pushed a commit to Leoyzen/sglang that referenced this pull request Aug 14, 2026
…root cause for sgl-project#33835) (sgl-project#34167)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 16, 2026
…root cause for sgl-project#33835) (sgl-project#34167)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@DarkSharpness

Copy link
Copy Markdown
Collaborator

Thanks for the PR. This should already be fixed by #34167. Feel free to reopen if that PR does not work in your case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants