Skip to content

feat(rdna2): default gfx1030 MoE accumulation to fp32 - #152

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

BlivionIaG wants to merge 1 commit into
rdna_extrasfrom
cursor/rdna2-moe-fp32-accum-default-0c31

Conversation

@BlivionIaG

Copy link
Copy Markdown

Overview

VLLM_RDNA2_MOE_FP32_ACCUM now defaults on for the gfx1030 fused MoE epilogue (fp32 scratch, one fp32→fp16 cast). =0 keeps the packed-fp16 CAS-64 path. Comments that called the fp32 path native v_global_atomic_add_f32 were wrong on gfx1030: that opcode is gfx11+, and atomicAdd(float*) lowers to CAS-32 retry loops.

No open opengfx1030/vllm-rdna PR changes this default (57 open PRs scanned for fp32 / accum / epilogue). The env var does not appear on open vllm-project/vllm PRs. This PR targets rdna_extras only.

AI assistance was used (Cursor Agent).

Claims

  • Unset, 1, and true select fp32 accumulation for moe_gptq_gemm_rdna2 (W4A16) and moe_w4a8_gemm_rdna2 (W4A8).
  • Opt out with VLLM_RDNA2_MOE_FP32_ACCUM=0 (also false) before vLLM is imported. A per-call fp32_accum=False still overrides one call.
  • Kernel ISA is unchanged. The fp32 kernel path already existed.

How to opt out

export VLLM_RDNA2_MOE_FP32_ACCUM=0

Recipes and tools/rdna/serve_rdna.sh do not set this variable, so the registry default is what serving uses. The production call sites omit fp32_accum and follow the env. The torch op schema default was already fp32_accum=True; the wrapper was the piece that still passed false.

Changed files

  • vllm/envs.py — annotation default True; getenv fallback "1".
  • vllm/_custom_ops.py — comment only. Still reads the env once at import.
  • csrc/rocm/moe_accum_rdna2.cuh — comments only.
  • csrc/rocm/moe_q_gemm_rdna2.cu — comments only.
  • csrc/rocm/moe_w4a8_rdna2.cu — comments only.
  • docs/rdna2/moe-epilogue-accum.md — default, opt-out, ISA, historical matrix labeled as historical.
  • docs/rdna2/tuning.md — env table default 1.
  • tests/test_envs.py — CPU test of unset / 1 / true vs 0 / false.
  • tests/kernels/quantization/test_rdna2_moe_w4a16.py — docstrings. The old "default off" test now says it forces the flag off.

docs/rdna/serving.md and the README do not mention this variable. Serving already points kernel defaults at docs/rdna2/tuning.md.

Validation

CPU parser, from a temp venv (CPU torch only so import vllm works; --noconftest because tests/conftest.py needs the full test extra). Fork pins torch==2.12.0+rocm7.14.0 and ROCm 7.14.0 are unchanged.

python -m pytest --noconftest tests/test_envs.py::test_rdna2_moe_fp32_accum_defaults_on -q

Result: 1 passed.

ISA check, host clang 18.1.3 (-target amdgcn-amd-amdhsa -mcpu=gfx1030 -O3 -nogpulib), not the ROCm 7.14 hipcc. No ROCm device library here, so vLLM was not rebuilt. An atomicrmw fadd of four floats (the moe_accum_row<float> shape, with !amdgpu.no.fine.grained.memory) emits four global_atomic_cmpswap loops at offsets 0/4/8/12 plus s_cbranch_execnz. No global_atomic_add_f32. The source calls atomicAdd(out + j, vals[j]) four times. The packed-fp16 path is the existing atomicCAS on unsigned long long in atomic_add_pk4_f16 (one CAS-64). The gfx1030 ISA has no v_global_atomic_add_f32 (gfx11+), so the ROCm pin cannot emit it either. Same conclusion as rdna-hip-wiki#45.

No gfx1030 GPU in this environment. tests/kernels/quantization/test_rdna2_moe_w4a16.py was not run. Those tests are @gfx1030_only and still monkeypatch both modes.

W4A16 q_gemm_rdna2* / fdot2, EXL3 exl3_dot2_*, w4a8_sdot4, and rdna_allreduce are untouched. No stub signature or arch-guard edit, so the gfx1030;gfx1100 split in docs/rdna2/build-break-2026-10-31.md is unchanged. That dual-arch build was not recompiled here.

V620 validation still required

Do not treat the new default as validated until a V620 MoE decode run under contention (small batch, top-k > 1, many experts writing the same output rows) reports both of the following against the fp16 opt-out (VLLM_RDNA2_MOE_FP32_ACCUM=0):

  • tok/s
  • correctness (greedy, or the existing MoE numeric tests)

The fp32 path is four CAS-32 retry loops per row; the current packed-fp16 path is one CAS-64. The CAS-32 loops may be slower. This PR claims no tok/s number. The 2026-09-30 matrix kept in docs/rdna2/moe-epilogue-accum.md is historical context only.

Details

The registry default was "False" while several kernel comments already called fp32 the default and described it as a native atomic with no CAS. Serving goes through _custom_ops, which resolves fp32_accum=None from that env at import, so the shipped path was packed-fp16 CAS-64.

The fp32 scratch, the cast kernel, and atomicAdd were already in tree. This change flips the default and corrects the comments. The env is a torch.compile factor, so an unset worker's compile-cache key changes with the resolved flag.

Design fit: no scheduler or model-runner change, no new kernel, opt-out preserved. Tradeoff: one fp16 rounding at the cast, against a possible decode slowdown under contention that still needs a V620 measurement.


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.

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing

Open in Web Open in Cursor 

VLLM_RDNA2_MOE_FP32_ACCUM now defaults on. Set it to 0 for the packed
fp16 CAS-64 epilogue. Comments no longer claim gfx1030 emits
v_global_atomic_add_f32; atomicAdd(float*) is a CAS-32 retry loop there.

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

Copy link
Copy Markdown
Author

Closing: this re-proposes the fp32 MoE-epilogue default, which was tried on 2026-09-30 and reverted to CAS on 2026-10-01. Recorded decision (AGENTS.md "MoE epilogue accumulation (default = CAS, 2026-10-01)", docs/rdna2/moe-epilogue-accum.md, bench_results/2026-09-30_fp32-accum-matrix/SUMMARY.md): CAS is +2.5 %/+2.6 % at MTP0 and +11.8 %/+13.4 % at MTP2 at c=1 (parity at c=8, FULL_AND_PIECEWISE on 4x V620); fp32 is the opt-in VLLM_RDNA2_MOE_FP32_ACCUM=1 (vllm/envs.py:1516).
The "gfx1030 has no native fp32 atomic" comment fix is already in csrc/rocm/moe_q_gemm_rdna2.cu:22-25. This PR has no V620 measurement of its own and 7 conflicts. Duplicate of #153; closing both.

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