fix(qmoe): compute down_proj (fc2) in fp32 to avoid fp16 overflow - #672
Draft
AMDmoore wants to merge 1 commit into
Draft
fix(qmoe): compute down_proj (fc2) in fp32 to avoid fp16 overflow#672AMDmoore wants to merge 1 commit into
AMDmoore wants to merge 1 commit into
Conversation
|
Thanks for opening a PR! This project follows LLVM's incremental-development and AI-tool-use Before requesting review, please check that:
Reviewers are assigned through |
A single expert's raw down_proj channel can exceed the fp16 max (65504) and overflow to inf BEFORE the routing weight scales it back into range, corrupting the MoE output (observed as NaN logits / PPL blow-up on gpt-oss-120b). Keep the fc2 GEMM result in fp32 until the weighted cross-expert accumulation, then narrow to fp16 once. - matmul_nbits_kernel.hip: template GemmFp16U4Impl and the two WMMA entry wrappers (MatMulNBitsWMMA_ZP/_NoZP) on OutT (default _Float16, so every existing fp16 instantiation is byte-identical). hip_matmul_nbits_fp32out now dispatches on M exactly like the fp16 path, with fixed configs (no autotune, no autotune cache touched): M>=16 && K%32==0 -> fused-dequant WMMA fp32 (needs fp16 zeros); otherwise -> row-major int4 GEMV fp32 (needs uint8 zeros). Only the final store narrowing changes. - hip_custom_kernels.h: add zero_points_fp16 param to fp32out. - qmoe.cpp (prefill/multi-token path): run fc2 into an fp32 scratch with no routing weight and no bias; prepare fp16 zeros when the WMMA path is reachable (count>=16 && inter%32==0). The scatter applies the routing weight to the full (GEMM + bias) result and accumulates across experts in fp32, so w_e*(W_dn.h_e + b_e) never rounds/overflows through fp16. Decode (num_tokens==1) uses the separate fused-decode kernel and is unchanged by this commit. Co-authored-by: Cursor <cursoragent@cursor.com>
L2 Accuracy Results (EP vs CPU)
Threshold: 0.01 | Run: 3701 - Commit: |
MorphiZen EP Performance Results
EPContext Export Performance
EPContext Import Performance
OGA Benchmark Results
OGA Wheel Smoke (Python benchmark_e2e.py)
Run: 3701 - Commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A single QMoE expert's raw
down_proj(fc2) channel can exceed the fp16 max (65504) and overflow toinfbefore the routing weight scales it back into range. Narrowing that raw GEMM result to fp16 at store time therefore corrupts the MoE output (observed as NaN logits / perplexity blow-up on gpt-oss-120b, e.g. layer 34).This PR keeps the fc2 GEMM result in fp32 until the weighted cross-expert accumulation, then narrows to fp16 exactly once — so
w_e * (W_dn·h_e + b_e)never rounds/overflows through fp16.What changed
matmul_nbits_kernel.hip— templateGemmFp16U4Impland the two WMMA entry wrappers (MatMulNBitsWMMA_ZP/_NoZP) on an output typeOutT(default_Float16, so every existing fp16 instantiation is byte-identical — only the final store narrowing changes; the fp32 accumulator was always fp32).hip_matmul_nbits_fp32outnow dispatches onMexactly like the fp16 path, with fixed configs (no autotune, no autotune cache touched):M >= 16 && K % 32 == 0→ fused-dequant WMMA fp32 (128×128, bounds-checked; needs fp16 zeros)hip_custom_kernels.h— addzero_points_fp16parameter tohip_matmul_nbits_fp32out.qmoe.cpp(prefill / multi-token path) — run fc2 into an fp32 scratch with no routing weight and no bias; prepare fp16 zeros only when the WMMA path is reachable (count >= 16 && inter % 32 == 0, identical to the launcher's WMMA gate so asym never silently falls back to zp=8). The scatter applies the routing weight to the full(GEMM + bias)result and accumulates across experts in fp32.qmoe_kernel.hip—scatter_add_fp32reads fp32 expert output and applies the routing weight to the whole(GEMM + bias); the old pre-scaling helper was removed.Scope / notes
num_tokens == 1) uses the separate fused-decode kernel and is unchanged by this PR.OutTdefaults preserve every existing instantiation.Test plan
4b4dfa98vs this branch.count(long prompt) and GEMV fp32 for smallcount.