cuda: fix routed MMVQ fusion dispatch for quantized mul_mat_id - #2
Open
rapatel0 wants to merge 1 commit into
Open
cuda: fix routed MMVQ fusion dispatch for quantized mul_mat_id#2rapatel0 wants to merge 1 commit into
rapatel0 wants to merge 1 commit into
Conversation
`ggml_cuda_mul_mat_vec_q` lowered routed MUL_MAT_VEC_FUSION into
`mul_mat_vec_q_switch_ncols_dst` with both `ids != nullptr` and
non-empty fusion args. Inside that switch, the dispatch unconditionally
took the dedicated `mul_mat_vec_q_moe` kernel whenever `ids != nullptr`.
The MoE kernel computes only the routed matmul — it does not apply
`x_bias`, `gate`, `gate_bias`, or GLU. So FUSION metadata was being
silently dropped on every routed dispatch.
This is reproduced by `test-backend-ops -p mxfp4` on the
MUL_MAT_VEC_FUSION case with `use_id=1`:
with_bias=1, with_gate=0 → small error (bias missed)
with_gate=1 → catastrophic error (entire GLU missed)
Both with_bias=0/with_gate=0 (no fusion) and non-routed FUSION
(use_id=0) pass because they avoid the affected code path.
Fix:
* `mul_mat_vec_q_switch_ncols_dst` now uses the dedicated MoE kernel
only when `ids != nullptr && !has_fusion`. Routed FUSION falls
through to the existing fusion-capable generic MMVQ kernel, which
already applies the main matmul, bias, gate matmul, gate bias, and
GLU activation.
* The generic `mul_mat_vec_q` kernel's routed single-column ID
indexing now reads `ids[channel_dst + token_idx*ids_stride]` and
sets `sample_dst = 0` in routed mode. This matches the existing
F16/BF16 fusion path in `mmvf.cu` and the dedicated MMVQ MoE kernel
layout, and corrects ID lookup when there is more than one token
worth of routing rows.
Validation on V100 (sm_70) via `test-backend-ops -p mxfp4`:
before: 80 failed records (76 MUL_MAT_VEC_FUSION + 4 misc)
MUL_MAT_VEC_FUSION use_id=1 worst err 8.15 vs 0.005 tol
after: 0 failed records / 1352 total
80 routed FUSION records (use_id=1) all pass
F8_E4M3_B128 still 0/12; no regression on other paths
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
ggml_cuda_mul_mat_vec_qwas lowering routedMUL_MAT_VEC_FUSIONinto the dedicatedmul_mat_vec_q_moekernel wheneverids != nullptr. That MoE kernel computes only the routed matmul — it does not applyx_bias,gate,gate_bias, or GLU. So FUSION metadata was being silently dropped on every routed dispatch.This affects every CC; it's a dispatch shortcut bug, not a code-gen issue.
Repro
test-backend-ops -p mxfp4onMUL_MAT_VEC_FUSIONwithuse_id=1:with_bias=1, with_gate=0with_bias=0, with_gate=1with_bias=1, with_gate=1Pattern explanation:
use_id=0): passes (different dispatch path)MUL_MAT_ID: passes (MoE-only kernel is correct for that case)Fix
Two changes in
ggml/src/ggml-cuda/mmvq.cu(8 insertions, 5 deletions):Dispatch shortcut:
mul_mat_vec_q_switch_ncols_dstnow uses the dedicated MoE kernel only whenids != nullptr && !has_fusion. Routed FUSION falls through to the existing fusion-capable generic MMVQ kernel, which already applies the main path, bias, gate path, gate bias, and GLU activation.Routed ID indexing: the generic
mul_mat_vec_qkernel's routed single-column path now readsids[channel_dst + token_idx*ids_stride]and setssample_dst = 0in routed mode. Matches the existing F16/BF16 fusion path inmmvf.cuand the dedicated MMVQ MoE kernel layout, and corrects ID lookup when there is more than one token worth of routing rows.Validation
Built + tested on V100 (sm_70) inside a CUDA 12.2 dev pod against this branch's HEAD:
MUL_MAT_VEC_FUSIONrecordsuse_id=1)MUL_MAT,MUL_MAT_ID(non-fused)test-backend-ops -p f8_e4m3_b128still reports 0 failures over 12 records.Test plan
test-backend-ops -p mxfp40 failures (verified)test-backend-ops -p f8_e4m3_b128no regression (verified)Notes
Found while bringing up DeepSeek-V4-Flash native FP4/FP8 (MXFP4 experts + F8_E4M3_B128 dense) on 4× V100 32GB hardware against a pruned 8-expert GGUF. Without this fix, MoE decode produces garbage on the routed FUSION path used for the expert FFN dispatch.
🤖 Generated with Claude Code