Repository navigation
[ROCm][Perf][MiniMax-M3.1] Fuse the FFN-side MXFP8 activation quant into the post-attention norm and swiglu - #65478
Fangzhou-Ai wants to merge 5 commits into
Conversation
…ion norm On ROCm the FFN's first projection (the dense MLP's gate_up_proj, or the MoE shared expert's) quantized its bf16 input with a separate `_kernel` launch right after the post-attention Gemma add+RMSNorm, once per layer per decode step. Give the Triton add+RMSNorm an MXFP8 epilogue that also writes the bf16 normed rows (the router and routed experts take bf16), and hand the quantized activation to the shared expert / dense MLP so its linear skips its own quant. The output is bit-identical to norm -> quant. It applies when the all-reduce is the plain collective followed by the Triton norm; when a fused all-reduce+norm kernel applies (no quant epilogue), the unfused path is kept. Builds on vllm-project#63637, which did the same for input_layernorm -> qkv_proj. Co-authored-by: RuibinCheung <ruibzhan@amd.com> Signed-off-by: Fangzhou Ai <fangzhou.ai@amd.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…quant The dense MLP and the MoE shared expert ran swiglu_oai and then quantized its bf16 output for down_proj with a separate `_kernel` launch. Use the existing `swiglu_oai_quantize_mxfp8` kernel (computes the activation in fp32 and quantizes it in the same pass) and hand the result to down_proj as a QuantizedActivation. It is skipped where down_proj already quantizes inside its GEMM (M <= 16 and K <= 1024: the shared expert at decode batches up to two requests), since that is a single launch as well. The fused kernel skips the bf16 rounding of the activation before the quant, so a few E4M3 codes differ from swiglu -> bf16 -> quant; they are closer to the fp32 activation (see the new test). Signed-off-by: Fangzhou Ai <fangzhou.ai@amd.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
selimfedakar
left a comment
There was a problem hiding this comment.
Thanks for this — it's an unusually well-documented PR, and the honesty in the
perf section (separating the solid launch-count/per-kernel numbers from the
end-to-end deltas that sit in the noise, and explicitly not claiming an e2e
gain) makes it much easier to review. I went through the diff against main
and traced the surrounding code; overall this looks correct and I'm in favor of
merging. No blocking issues. A few questions and nits below, plus the things I
specifically checked and found safe.
Context: we've been working the MXFP8 activation scheme invariant on the
linear side (the native MXFP8 kernel must honor the activation scheme:
self-quantizing kernels advertise kMxfp8Dynamic with row-major E8M0 scales,
weight-only stays BF16). This PR is a producer for exactly that contract, so I
paid most attention to whether the fused epilogues emit activations the consumer
actually expects.
What I checked and found safe
- Norm epilogue is bit-identical to the unfused path. The new
ALSO_BF16
branch ingemma_rmsnorm.pystores the normed row before theQUANT_MXFP8
block and still re-casts throughout.to(x_ptr.dtype.element_ty), so the FP8
values and E8M0 scales are computed from the same input-dtype-rounded row as
norm -> mxfp8_e4m3_quantize. The bitwise-eq test covers bf16/fp16 and N in
{2048, 6144}. - Scale layout / scheme contract matches the consumer. The fused ops emit
row-major[M, N//32]E8M0 scales (not swizzled) underkMxfp8Dynamic, which
is exactly whatRocmDotScaledMxfp8LinearKernelconsumes (row-major, not
swizzled). The gate_can_fuse_input_norm_mxfp8requires that kernel +
supports_quantized_input(...), so a FlashInfer-style consumer that wants
swizzled scales under the same key can never receive these — the same
layout-vs-key distinction matters here and it's handled correctly. down_projin-GEMM-quant carve-out reproduces the kernel's own decision
(0 < M <= _FUSED_QUANT_MAX_M and _select_fused_quant_cfg(...) is not None),
so no double-quant / no missed fusion; the test pinsin_gemm == (m<=16 and k<=1024).- Prequant hand-off is fail-safe:
forwardonly substitutes when the
incomingxis a tensor with matchingdata_ptr()andshape, consumes it
once, else falls back to self-quantization. DP/EP paths fall back cleanly. - Batch-invariant mode is fully disabled (
_can_fuse_input_norm_mxfp8
returns False underVLLM_BATCH_INVARIANT), so thedown_projpath not
re-checking it is harmless. None-deferral keeps the fused-AR / N=1 path byte-for-byte unchanged.- Perf claims appropriately scoped (per-launch savings + exact launch-count
reduction are the credible core; e2e correctly not claimed).
Questions (non-blocking)
- The fused swiglu path is numerically different (not bit-identical) by
design — it quantizes the fp32 activation directly and skips the bf16
round-trip the unfused chain does (~1.9-2.4% of E4M3 bytes differ, and your
test only bounds it as at least as close to the fp32 ref). The op documents
this, but the substitution at theMiniMaxM3MLP.forwardcall site is silent —
a one-line comment there would save the next reader the trip. - The 6 failing kernel tests (
moe_align_block_sizemissing) reproduce on the
base commit too, i.e. a test-machine extension gap, not a regression — agreed,
but worth confirming they pass on a real gfx950 CI image before merge. return_normed=self.is_moe_layer: confirming the dense path has no consumer
of the post-attn normed output other thangate_up_proj(which takes MXFP8
directly), so discardingnormedfor dense is safe.
Nits
_gemma_fused_add_rmsnorm's return-type annotation was dropped when
also_bf16was added (now a 3- or 4-tuple) — worth restoring._SWIGLU_QUANT_BLOCK_M = 16— note it was tuned on gfx950 specifically (the
MoE default is 64).- PR body says swiglu fusion applies to dense "always"; the code correctly also
skips it when_down_proj_quantizes_in_gemm— minor wording.
Correct, well-tested, and the activation-scheme/scale-layout contract lines up
with the native MXFP8 consumer. The guards all fail safe toward the unfused
path. I'd approve once the swiglu-not-bit-identical comment (Q1) is added and
the two extension-gap failures are confirmed green on real gfx950 CI. Nice work.
…ment tuning/numerics Add overloads on also_bf16 so callers type-check, note that the swiglu quant block size is tuned on gfx950 only, and document that the fused swiglu quant is intentionally not bit-identical to the unfused chain. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Fangzhou Ai <fangzhou.ai@amd.com>
|
Thanks for the review, addressed in 6c4a595:
|
Take the clone before the 'is' asserts narrow x to QuantizedActivation. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Fangzhou Ai <fangzhou.ai@amd.com>
|
/ci run |
|
✅ Triggered Buildkite CI #99726 for commit |
ChuanLi1101
left a comment
There was a problem hiding this comment.
LGTM. Fusing the FFN-side MXFP8 quant into the post-attn norm / swiglu epilogues is the right producer for the native MXFP8 linears, and the dense vs MoE bf16-norm handling looks correct. Thanks for keeping the e2e claim honest. Selim’s asks look addressed.
Purpose
On ROCm, MiniMax-M3.1 decode runs one separate Triton
_kernellaunch (the MXFP8 activation quant,_mxfp8_quant_triton_kernel) in front of several MXFP8 linears. #63637 fused it intoinput_layernormforqkv_proj. Two more per-layer launches remain on the FFN side:gate_up_proj(dense MLP, or the MoE shared expert), every layer, every N.down_proj, every layer when the GEMM does not already quantize in-kernel (decode batches with more than 2 requests, i.e. M = 8N > 16). Dense layers skip the separate quant in the same way when the in-GEMM quant applies.This PR (two commits) turns both into epilogues of kernels that already run:
_gemma_fused_add_rmsnorm_kernelgets anALSO_BF16mode: it writes the MXFP8 values/scales and the bf16 normed rows (the router and routed experts need bf16). Newallreduce_gemma_rms_norm_mxfp8(all-reduce + that norm) returnsNonewhen a fused all-reduce+norm kernel applies, so that path is untouched. The MoE passes the quantized rows to the shared expert throughset_prequant(checked by data pointer + shape, consumed once).swiglu_oai_quantize_mxfp8(fp32 swiglu + quant in one pass) and givedown_projaQuantizedActivation; skipped where the GEMM already quantizes in-kernel (M <= 16, K <= 1024).Related: #63637 (same idea for input_layernorm -> qkv_proj); the fp32 swiglu kernel is from the MiniMax-M3 AMD ops. Not covered by any open PR that I could find.
What changes per decode step (main model, 60 layers)
Trace evidence (torch profiler, TP4, DSpark 7-token drafts so M = 8 x requests; rank-0 profile windows at N=1, 2, 4 and 16):
_kernel.kdper step = 145 at N=1 and N=2, 202 at N>=4 (profiled durations ~1.6x inflated).Still unfused after this PR: the
o_projinput quant (K=2048; in-GEMM quant did not win above M=16 in my sweep), layer 0's input norm, and the DSpark draft layers.Kernel-level measurements (MI355X, gfx950, 1 GPU, CUDA-graph replay of 200 back-to-back launches; us per launch)
Benchmark script below (collapsed), MiniMax-M3.1 TP4 shapes: hidden 6144, shared-expert intermediate 768.
(second run on the production code of this branch, GPU 6; an earlier run on GPU 7 agreed within 0.1 us.)
Microbenchmark script (CUDA-graph replay of 200 back-to-back launches)
PROJECTION made before the step-level runs (NOT a measurement, and about 2x too high compared with the A+B-only arm below, which averaged -0.7% over N and about -0.1 to -0.4 ms at N>=3): A+B remove 0.22-0.32 ms/step at N>=3 (~1.8-2.3%), nothing at N=1.
Test Plan / Correctness
mxfp8_e4m3_quantizeat all M in the table above (edge-case rows: all-zero, tiny, dominated and partly-zero blocks in the unit test).tests/kernels/test_minimax_m3_amd_ops.py(norm variant bitwise,allreduce_gemma_rms_norm_mxfp8for TP 1/4 and deferral to a fused AR kernel, prequant hand-off, gate_up/down_proj on the fused outputs, swiglu+quant vs unfused). Run on MI355X (gfx950):pytest tests/kernels/test_minimax_m3_amd_ops.py-> 128 passed, 6 failed. The 6 failures aretest_mxfp8_native_moe/test_mxfp8_grouped_gemm_nativewithAttributeError: '_moe_C' has no attribute 'moe_align_block_size'(the installed_moe_Cextension of the test machine lacks the op); they fail identically on the unmodified base commit c1d8fad.Launch counts and step-level measurements (MI355X, TP4, DSpark, 150K context)
All server settings as the AgentX recipe with DBO off and
VLLM_ROCM_USE_SKINNY_GEMM=0;decode_bench.py(3 repeats per arm, p50 decode-step time; run-to-run noise about +-0.1-0.2 ms, ~2%). Torch-profiler window of 60 steps, rank 0, all streams.Exact launch-count reduction (profile window, N=4 and N=16 identical):
_kernel.kd(the MXFP8 activation quant) 202 -> 82 per decode step (-120 = 60 layers x 2). Total kernels per step in the A+B-only arm: 1348 -> 1228 on the main stream (-120). What remains of_kernel.kd: theo_projinput quant (60), layer 0's input norm quant and the draft layers (22).A+B only (this PR's two commits on top of the installed tree), decode-step p50, ms:
Average over N=1..16: about -0.7%. Only N=3 (-3.1%) and N=16 (-2.3%) are outside the noise band; the rest is within it, and the sign is not monotonic in N. I therefore do not claim a specific end-to-end gain for this PR; what is established is the exact launch removal and the per-launch kernel savings above (1.6-3.3 us for the norm epilogue, ~2.0 us for the swiglu fusion). At N=1 and N=2 the PR is expected to do little (see Limitations).
Full stack (this PR + two other, separate changes: folding the routed scaling into the AITER top-k, and launching the shared expert after the routed chain in the HIP graph). Decode-step p50 improved at every N (-0.4% to -4.6%, mean -1.7%; N>=3 mean -0.17 ms); this is not attributable to this PR alone and is listed only for context. A repeat with 5 repetitions at N=1 and N=2 confirmed the stack's N=1 gain (9.22 -> 8.96 ms, -2.8%) and N=2 (10.20 -> 9.93, -2.6%); the A+B-only arm shows nothing at N=1/N=2 (-0.2%/+0.6%), and the scale-fold-only arm accounts for about -2.1% / -1.0% there. The full-stack N=1 gain is not additive across the single-change arms and I could not explain it.
Quality
Full stack (not this PR alone), GSM8K, all 1319 test questions, chat mode with reasoning allowed, greedy, real (not synthetic) draft acceptance: strict match 0.9363 (installed) vs 0.9424 (stack), flexible 0.9356 vs 0.9416; binomial SE about 0.008, i.e. within the noise. Outputs are not bit-reproducible even within one tree (spec decode, batching), so only accuracy statistics are compared. "What is 17*23?" -> 391 on both. The fused norm output is bit-identical to norm -> quant (unit tests and microbench); the swiglu fusion changes 1.9-2.4% of FP8 bytes (see above), which this GSM8K run cannot isolate.
AgentX replay: not run.
Limitations
allreduce_gemma_rms_norm_mxfp8.RocmDotScaledMxfp8LinearKernelconsuming row-major E8M0 scales (same condition as [ROCm][MiniMax-M3.1] Fuse MXFP8 activation quant into the Gemma input_layernorm for qkv_proj #63637).AI assistance: this change was developed with Claude Code; the tests and benchmarks above were run on MI355X (gfx950).
🤖 Generated with Claude Code