[SM120] Allow fused MHC opt-in with standalone TileLang pre disabled - #30954
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
@JoeLee314 This fixes SM120 activation of the fused path added in #25976. Could you review and trigger |
#25976 was submitted by me, but I’m only a contributor and don’t have permission to trigger /tag-and-rerun-ci. Could you ask a maintainer to review it and trigger /tag-and-rerun-ci extra? |
|
Measured this on SM120 hardware (2x RTX PRO 6000 Blackwell, TP2, DeepSeek-V4-Flash, MTP2 EAGLE steps=2/topk=1/draft-tokens=3, FP8 KV, FlashInfer MXFP4 MoE, full decode CUDA graphs, temperature 0). Five consecutive 30 s C1 runs per cell on a settled server, medians below:
Reproducibility note: rows 3 and 4 required a local patch wrapping the SM120 defaults in The second row is what this PR unlocks: +8.9% over the SM120 default, without needing to force Profiler traces explain the mechanism. Median target-verify decode graph, first row vs second:
Per-category deltas (fused minus non-fused): CUTLASS GEMM -767 us (492 -> 407 kernels), other GEMM -320 us (148 -> 63), norm -165 us (130 -> 45), fill -97 us, misc -434 us; quant +68 us and sparse attention +74 us. With Two observations for reviewers:
|
|
@ch-wan @Fridge003 review request when you have a moment. @ch-wan, you merged #25976 which added the fused MHC post/pre path. This is a three-line fix to its enable gate: on SM120 Measured impact on SM120 hardware is in the comment above: +8.9% C1 decode over the SM120 default (172.78 -> 188.16 median of five runs), with the profiler showing 510 fewer kernel nodes and 1.64 ms less summed kernel time per decode graph. Default behavior on other architectures is unchanged, and a unit test covers the policy matrix. |
mmangkad
left a comment
There was a problem hiding this comment.
The gate comment only covers decode (prefill uses different kernels), and the test is missing the __main__ block CI needs to run it
SM120 disables the standalone TileLang mhc_pre path during argument resolution. The fused post/pre gate also required that flag, so SGLANG_OPT_FUSE_MHC_POST_PRE was silently ineffective even though the fused FMA kernel is a separate supported path. Keep the existing TileLang-post requirement and bypass only the standalone-pre requirement on SM120. Signed-off-by: David Orman <ormandj@corenode.com>
Signed-off-by: David Orman <ormandj@corenode.com>
8ff2a9f to
07d2f68
Compare
Replace the gate comment's split-K rationale, which was inaccurate: the split-K TileLang kernel in mhc_pre is only reached for num_tokens <= 2048, and split-K is not itself unsupported on SM120 (mhc_fused_post_pre_fma_tilelang uses a dynamic split_k). State only what the code does instead. Add the unittest __main__ block so CI can execute the registered test.
07d2f68 to
cb1aa42
Compare
|
Thanks @mmangkad. Both addressed. The gate comment now states only what the code does, and the PR body is updated to match. The test has On your prefill note: is the concern comment accuracy only, or should the gate itself distinguish prefill from decode? Rebased onto current main. |
Co-authored-by: Mohammad Miadh Angkad <176301910+mmangkad@users.noreply.github.com>
|
@ormandj how did you test the |
|
Via The two rows the PR rests on do not need that patch: The same unconditional |
|
I also patched the override but hit a compilation error. Anyway, it is not reachable on main or needed here, so this looks good. |
mmangkad
left a comment
There was a problem hiding this comment.
Verified on SM120 with an A/B/A test and saw a repeatable 6%+ gain with the expected fused kernel launches
…bled (sgl-project#30954) Signed-off-by: David Orman <ormandj@corenode.com> Co-authored-by: Mohammad Miadh Angkad <176301910+mmangkad@users.noreply.github.com>
…bled (sgl-project#30954) Signed-off-by: David Orman <ormandj@corenode.com> Co-authored-by: Mohammad Miadh Angkad <176301910+mmangkad@users.noreply.github.com>
…bled (sgl-project#30954) Signed-off-by: David Orman <ormandj@corenode.com> Co-authored-by: Mohammad Miadh Angkad <176301910+mmangkad@users.noreply.github.com>
Motivation
SM120 model post-processing sets
SGLANG_OPT_USE_TILELANG_MHC_PRE=False._is_fused_mhc_post_pre_enabled()also required that flag, soSGLANG_OPT_FUSE_MHC_POST_PRE=1had no effect on SM120.A 60-step C1 trace with the fusion flag set contained, per decode cycle, 86
mhc_post, 86hc_split_sinkhorn, and 86hc_combinelaunches, and zeromhc_fused_post_pre_fma_tilelanglaunches.mhc_fused_post_prenever readsSGLANG_OPT_USE_TILELANG_MHC_PRE. It dispatches on its own token-count threshold, takingmhc_fused_post_pre_fma_tilelangfor small batches and a separate branch for large ones, selecting the GEMM there onSGLANG_OPT_DEEPGEMM_HC_PRENORM. Both regimes are handled independently of the standalone pre path, so that flag should not gate the fusion opt-in.Modifications
This follows the fused path added in #25976. It does not change the fused kernel or its numerical behavior.
Accuracy Tests
test/registered/unit/models/test_deepseek_v4_fused_mhc_policy.py): 3 cases pass.Effect
Hardware: 2x RTX PRO 6000 Blackwell Max-Q at 300 W, TP2, DeepSeek-V4-Flash, FP8 KV, FlashInfer MXFP4 MoE, full decode CUDA graphs, EAGLE MTP2.
With
SGLANG_OPT_USE_TILELANG_MHC_PREat its SM120 default (off), the non-fused path falls back tohc_pre_torch_impl, a torch-levelF.linear/rsqrtimplementation. The fusion opt-in replaces it. Median target-verify decode graph:The 510-node difference is the
hc_pre_torch_implfallback. Per-category (fused minus non-fused): gemm_cutlass -767 us (492 -> 407 kernels), other -434 us, gemm_other -320 us (148 -> 63), norm -165 us (130 -> 45), fill -97 us, quant +68 us, sparse_attention +74 us.Kernel-node counts are structural and do not vary run to run. Throughput is not reported here: the available A/B ran with EAGLE MTP2, whose acceptance rate drifts run to run, and the fused cell spanned 6.9% across five 30 s samples. A non-speculative re-measurement under the settled harness is pending.
Checklist
pre-commit run --filespassed on both changed files.test/registered/unit/models/.CI States
Latest PR Test (Base): ✅ Run #30159293986
Latest PR Test (Extra): ❌ Run #30159293903