Skip to content

Cuda mmq moe oob fix - #28383

Closed
Herculians wants to merge 4 commits into
ggml-org:masterfrom
Herculians:cuda-mmq-moe-oob-fix
Closed

Herculians wants to merge 4 commits into
ggml-org:masterfrom
Herculians:cuda-mmq-moe-oob-fix

Conversation

@Herculians

@Herculians Herculians commented Sep 4, 2026 •

Copy link
Copy Markdown

What is not working:
Fresh server > first request (web UI vision chat, system prompt + one 1024×1536 PNG + short prompt: 743 prompt tokens) dies instantly.

E ggml_cuda_compute_forward: SWIGLU_CLAMP failed
E CUDA error: an illegal memory access was encountered
E   current device: 1, ... at ggml-cuda.cu:2417

Bug hunting:
SWIGLU_CLAMP is a sticky-error misattribution (async kernels; first checkpoint after the fault reports it).
CUDA_LAUNCH_BLOCKING=1 > names MUL_MAT_ID as the faulting op cuda-gdb with CUDA_DEVICE_WAITS_ON_EXCEPTION=1 > fault at mmq.cuh:926, tile_y[l] = by0[l], CUDA_EXCEPTION_30 Warp MMU Fault = global MMU fault, not shared memory.

mul_mat_q_process_tile staging loop copies a full J-wide tile from the y buffer regardless of valid columns.
When a MoE expert's rows end at the buffer end, the read walks past the mapped region > Warp MMU Fault > sticky illegal-memory-access. Values past col_diff are never used, so it's a silent overread unless it crosses the pool's 2 MiB VMM mapping boundary.

Proposed fixes:

  • clamp staging reads to valid columns
  • clamp ids_dst reads including stream-k fixup
  • get_J_max floor for ne11 < 8 (the intended host-side tail ggml_cuda_mmq_get_J_max(...)*sizeof(block_q8_1_mmq) returns 0 for ne11 == 1 (broadcast MoE gate/up) > no tail at all)
  • NVFP4 y_scale tail

Verified:
10/10 pre-fix crashes, now 7/7 clean
test-backend-ops 3464/3464 (RTX 4090) and 2199/2199 (RTX 5090)
perf neutral-to-positive (386 > 360 microseconds on the n=512 case)

Environment:

Item Value
OS / driver Arch Linux, driver 610.57.04, CUDA 13.3
GPU0 RTX 5090, 32 GB, sm_120
GPU1 RTX 4090, 24 GB, sm_89
llama.cpp master c5a5535e6 (build 10792, tag b10792)
Build cmake -B build -DGGML_CUDA=ON -DCMAKE_CUDA_ARCHITECTURES="120;89" + cmake --build build --config Release
Model DeepSeek-V4-Flash-Vision-Exp - only FP8 converted to Q8_0, MXFP4 and other weigths are intact.
Server flags --device CUDA0,CUDA1 --split-mode layer --n-gpu-layers all --tensor-split 1,0 --override-tensor "blk\.3[0-5]\..*_exps=CUDA0,blk\.(3[6-9]|4[0-2])\..*_exps=CUDA1,ffn_.*_exps=CPU" --mmproj /path_to_model

Additional information

I found two stale-closed reports for the likely the same cause: #24174 and #25721
You unable to reproduce the #25721 because it is layout-dependence mechanism. The pool/layout state need to be produced by the model load and the specific allocation sequence of this request's ubatches. For example, with default --ubatch-size 512, the 743-token prefill splits into ubatches 512 + 231 and 231-token ubatch crashes.
Staging loop is shared by the Blackwell native-FP4 path and the dense ne11 < 8 path.

Requirements

@Herculians
Herculians requested review from a team and ggerganov as code owners September 4, 2026 11:31
@github-actions github-actions Bot added testing Everything test related ggml changes relating to the ggml tensor library for machine learning CUDA Related to the CUDA backend labels Sep 4, 2026
@Herculians
Herculians force-pushed the cuda-mmq-moe-oob-fix branch 2 times, most recently from 8ae8da3 to 1e71507 Compare September 5, 2026 11:11
The tile_y staging loop always reads a full J-wide tile from the y buffer.
For MoE experts whose rows end at the end of the buffer this reads up to
J-1 rows past the end of the allocation and can crash with an illegal
memory access. Clamp the read to the valid columns of the tile, rows past
them are never used.

Assisted-by: GLM-5.3
The ids_dst buffer only has col_diff valid entries for each expert but is
read J-wide in both MoE branches of mul_mat_q and in the stream-k fixup
kernel. The values past col_diff are never used, do not read them.

Assisted-by: GLM-5.3
ggml_cuda_mmq_get_J_max returns 0 when ne11 < 8, for example for MoE
gate/up projections with broadcast activations, leaving no padding for
the J-wide tile reads. Return the smallest valid J instead. Also pad the
NVFP4 y_scale allocation, the stream-k fixup reads it J-wide.

Assisted-by: GLM-5.3
256 experts, top-6, m=2048, k=4096, skewed expert distribution and
broadcast activations, as produced by MoE gate/up projections.

Assisted-by: GLM-5.3
@JohannesGaessler

Copy link
Copy Markdown
Contributor

Can you check whether #29941 fixes these issues?

@Herculians

Copy link
Copy Markdown
Author

#29941 overlaps with my host-side padding for the ids path only.
The ids_dst clamps, the dense-path get_J_max floor, the staging clamp, and the NVFP4 y_scale tail remain necessary. My PR is still needed, possibly rebased with the ne11→ne12 hunk dropped as superseded by your #29941

@JohannesGaessler

Copy link
Copy Markdown
Contributor

Sorry, I forgot to mention: there were more issues found, see #29953 . In general these out-of-bounds issues should not be solved using explicit checks in device code since the performance is quite sensitive to those. So I would really not want to merge changes like in this PR if at all possible.

@Herculians

Copy link
Copy Markdown
Author

Test set: MXFP4 DeepSeek-shape MUL_MAT_ID cases + existing nvfp4/q8_0 ones, RTX 5090.

Build OOB errors Tests
master + #29941 only 937 → 1471 (+158 dense small-batch) abort
#29941 + my 3 fixes 0 all pass
#29953 0 240/240 pass

#29941 does not fix my issues, but #29953 does!

Remaining questions/proposals:
#29953 never pads src1_scale (my NVFP4 y_scale +128 point). Should we?
I believe that my contribution to tests/test-backend-ops.cpp (test_mul_mat_id with a skewed expert distribution) is still valuable, consider to borrow in your #29953.

@JohannesGaessler

Copy link
Copy Markdown
Contributor

Sorry, but I think we should be careful with adding new structs and large tests to test-backend-ops.cpp; I think there's a better way to detect these issues.

It should not be necessary to pad src1_scale because the memory access is conditional and only happens in ggml_cuda_mmq_write_back_mma rather than proactively at the beginning of a tile.

I'm closing this PR in favor of the other one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA Related to the CUDA backend ggml changes relating to the ggml tensor library for machine learning testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants