Skip to content

feat(rdna2): default the MoE quantized GEMM epilogue to fp32 - #153

Closed
BlivionIaG wants to merge 1 commit into
rdna_extrasfrom
cursor/moe-fp32-accum-default-cd87
Closed

BlivionIaG wants to merge 1 commit into
rdna_extrasfrom
cursor/moe-fp32-accum-default-cd87

Conversation

@BlivionIaG

Copy link
Copy Markdown

Overview

VLLM_RDNA2_MOE_FP32_ACCUM now defaults on, so the RDNA2 W4A16/W4A8 MoE epilogue accumulates in an fp32 scratch and casts to fp16 once. VLLM_RDNA2_MOE_FP32_ACCUM=0 still selects the packed-fp16 CAS-64 path. Comments that called the fp32 atomic a native v_global_atomic_add_f32 now match the ISA.

Not a duplicate. Open PR #93 is the opt-in W8A8 sdot4 kernels, and #87 deletes old campaign drivers. No open issue tracks this default.

Claims

Validation

CPU only. This environment has no GPU, so the gfx1030 kernel tests were not run.

# fails on origin/rdna_extras (unset getter returns False)
# passes on this branch
python -m pytest --noconftest \
  tests/test_envs.py::test_rdna2_moe_fp32_accum_defaults_on -q

On the previous getter the new test failed at the unset case (assert False is True). On this branch it passed (1 passed). ruff check and ruff format --check passed on vllm/envs.py, vllm/_custom_ops.py, tests/test_envs.py, and tests/kernels/quantization/test_rdna2_moe_w4a16.py.

tests/kernels/quantization/test_rdna2_moe_w4a16.py still monkeypatches _RDNA2_MOE_FP32_ACCUM for both epilogues. test_fp32_accum_flag_off_byte_identical_to_cas is the old "default off" case, renamed so it no longer describes the unset default. It was not executed here.

No model eval. The default changes MoE outputs at the last fp16 ULPs.

Details

The shipping default is the env getter, snapshotted once at import by vllm/_custom_ops.py (_RDNA2_MOE_FP32_ACCUM). Both the runtime lambda and the TYPE_CHECKING annotation now default to true (os.getenv(..., "True")).

Already consistent, left alone:

  • csrc/rocm/torch_bindings.cpp already defaults fp32_accum=True on both ops. The schema was not edited, so the dual-arch gfx1030;gfx1100 _rocm_C symbol list stays as fix(rocm): unbreak the gfx11 and host builds — four drifted stubs/guards at b740289353 #150 left it.
  • The host functions take the bool from the caller. They do not have a second default.
  • tools/rdna/serve_rdna.sh and tools/rdna/recipes/*.env do not set the variable, so a recipe launch follows the code default.

atomicAdd(float*) is not an explicit v_global_atomic_add_f32. gfx1030 has no global f32 atomic add, so that call compiles to a compare-and-swap loop. The instruction exists on gfx11; a gfx1100 compile of the helper may emit it. These kernels' device bodies stay gfx1030-only. The gfx1100 image is still the empty stub.

Expected cost. On gfx1030 the fp32 path is a CAS-loop atomic into a scratch plus a cast, and it can be slower than packed-fp16 CAS-64, in exchange for one rounding at the end instead of an order-dependent fp16 atomic. The 2026-09-30 V620 Flash-Next matrix (bench_results/2026-09-30_fp32-accum-matrix/SUMMARY.md, 4× V620, TP=4, FULL_AND_PIECEWISE) measured CAS faster by about +2.5% decode tok/s at MTP=0 c=1 and +12–13% at MTP=2 c=1, with c=8 near parity. That is not a measurement of this tree.

Please measure decode tok/s and worst-case ITL on V620 before and after, on the recipe you serve (16k/1k and 1k/512, c=1 and c=8). Revert with:

VLLM_RDNA2_MOE_FP32_ACCUM=0

The variable is a torch.compile factor, so the first serve after the flip recompiles graphs.

AI assistance was used (Cursor Agent). A human should review every line and run the gfx1030 MoE tests plus the V620 A/B above before relying on the new default.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

Open in Web Open in Cursor 

VLLM_RDNA2_MOE_FP32_ACCUM now defaults on. =0 still selects the
packed-fp16 CAS-64 path. Comments state that gfx1030 lowers the fp32
atomic to a CAS loop and that gfx1100 may use v_global_atomic_add_f32.

Co-authored-by: BlivionIaG <BlivionIaG@users.noreply.github.com>
@BlivionIaG

Copy link
Copy Markdown
Author

Closing: same change as #152 (default the MoE epilogue to fp32), rejected by the CAS default decision of 2026-10-01 (AGENTS.md "MoE epilogue accumulation", docs/rdna2/moe-epilogue-accum.md:103-125: CAS +2.5-13.4 % at c=1, parity at c=8; fp32 stays opt-in via VLLM_RDNA2_MOE_FP32_ACCUM=1, vllm/envs.py:1516). It conflicts in the same 7 files and carries no new V620 measurement. See the comment on #152.

@BlivionIaG BlivionIaG closed this Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants