[Blackwell] Reserve SMs for DeepGEMM MegaMoE grid barriers - #36657
Conversation
4487614 to
02b0430
Compare
|
/tag-and-rerun-ci |
02b0430 to
0b3393b
Compare
| with mega_stream_ctx: | ||
| import deep_gemm | ||
|
|
||
| with ( |
There was a problem hiding this comment.
Could we scope this SM override to the actual deep_gemm.fp8_fp4_mega_moe call?
set_num_sms is process-wide, and the current context also covers the gate, top-k, buffer setup, and pre-dispatch. Any other DeepGEMM call in this path would inherit the reduced SM count, which seems broader than this fix requires.
There was a problem hiding this comment.
make sense, changed to only guard fp8_fp4_mega_moe
| num_topk, | ||
| hidden, | ||
| intermediate_hidden, | ||
| num_sms, |
There was a problem hiding this comment.
Does the symmetric buffer actually depend on num_sms?
In sgl-deep-gemm 0.1.5.post3, its size and layout are derived from the rank, token, and model dimensions, not the active SM count. If there is no hidden dependency, can we remove this cache-key field and the related argument plumbing? Otherwise changing the reserve can allocate and rendezvous another large symmetric buffer unnecessarily.
There was a problem hiding this comment.
thanks, this num_sms won't change after setting anyway, changed.
|
The fix direction makes sense. Before merging, could we carry the Blackwell validation from #30592 over to the current PR head? At minimum, I would like to see:
The current PR only reports CPU unit tests, while this changes a GPU liveness condition and the default MegaMoE launch size. |
0b3393b to
9401f06
Compare
Root cause: Blackwell MegaMoE launches an even clustered grid that participates in a whole-grid software barrier. When another stream occupies an SM, launching one cluster per available device SM can leave part of the grid non-resident and trigger a grid-sync timeout. Fix: Reserve two SMs by default only while launching each Blackwell fp8_fp4_mega_moe kernel, round the remaining count for two-CTA clusters, and restore DeepGEMM's process-wide setting even when launch setup raises. Apply the same scoped protection to the DeepSeek and Kimi K3 MegaMoE call sites while leaving buffer selection and the SM90 path unchanged. Validation: - python3 compileall for the modified source and test - pre-commit run --all-files --show-diff-on-failure
9401f06 to
06b5da5
Compare
|
@BBuf thanks for review, test is updated on pr decripetion. |
…ct#36657) Co-authored-by: weireweire <20922698+weireweire@users.noreply.github.com>
…ct#36657) Co-authored-by: weireweire <20922698+weireweire@users.noreply.github.com>
Summary
Root cause
Blackwell MegaMoE uses an even clustered grid with a whole-grid software barrier. If the launch uses every available SM while another CUDA stream temporarily occupies an SM, not all clusters can become resident. Resident clusters then wait at the barrier for clusters that cannot launch, eventually producing a DeepGEMM grid-sync timeout and a subsequent CUDA launch failure.
Fix
For SM100 and newer, derive the MegaMoE launch size from DeepGEMM's current SM count and reserve two SMs by default. Round the active count down to an even number so it remains compatible with the two-CTA cluster.
Apply the override only around the DeepSeek and Kimi K3
deep_gemm.fp8_fp4_mega_moecalls, then restore DeepGEMM's process-wide setting even when launch setup raises. Gate computation, top-k selection, symmetric-buffer setup, and pre-dispatch continue to use the original setting. The symmetric-buffer cache key remains based only on parameters that affect buffer size or layout.The separate SM90 MegaMoE implementation remains unchanged. This supersedes the closed #30592 with a smaller diff and explicit architecture gating.
Fixes #30399.
Validation
CI States
Latest PR Test (Base): ⏳ Run #33208972574
Latest PR Test (Extra): ⏳ Run #33208972664
Latest PR Test (AMD ROCm 7.2): ⏳ Run #33208972751