Skip to content

docs(upstream): llama.cpp#27044 was reviewed; test the maintainer's variant on sm_120 - #445

Merged
glennneuber merged 4 commits into
mainfrom
docs/mmq-upstream-review
Oct 4, 2026
Merged

glennneuber merged 4 commits into
mainfrom
docs/mmq-upstream-review

Conversation

@glennneuber

Copy link
Copy Markdown

A hand-off to the CUDA host (ai-server/mlx-cuda), on the maintainer's word. Our upstream MMQ fix, ggml-org/llama.cpp#27044, has been reviewed. The reply needs tests on sm_120, the hardware of the original crash, under compute-sanitizer.

What happened upstream (2026-10-04)

  • The upstream CUDA maintainer wrote #24127, the refactor that introduced the bug. He said our fix "looks 90% correct" and opened his own one-line PR, #29941. He asked whether it also works for us.
  • #29941 pads with ne12 instead of our ne12*n_expert_used.
  • An upstream reviewer linked issue #29847, which has a test-backend-ops reproducer, and suggested adding its two cases to #27044.

What this PR records

docs/maxusai/upstream-mmq-submission-material.md gets a section with the facts:

  • From 512 tokens up, both lines give the same padding. That covers our crash (2040 tokens) and every crash reported on #27044.
  • Below 512 tokens, #29941 can pad too little. This comes from reading upstream master, not from a measurement:
    • the padding helper rounds down to a multiple of 8;
    • the launch rounds the tile width up;
    • the src1 tile load has no bound.
  • Compat 903 carries our line. If upstream merges #29941 instead, 903 stops applying at the next llama.cpp bump.

For the CUDA host

  1. On sm_120, under compute-sanitizer --tool memcheck, run master, master + 903 and master + #29941 on these cases:
    • our original reproducer (a 2040-token ubatch);
    • #29847's two test_mul_mat_id cases;
    • optionally, a case below 128 tokens that is not a multiple of 8, for the rounding gap.
  2. Post the results here, as a comment or as a commit on this branch.
  3. Do not post on #27044 or #29941. llama.cpp prohibits AI-written posts, replies included (CONTRIBUTING.md; see the top of the record). The maintainer writes the reply from the results.

Checks

  • check_source_paths.py --changed-since origin/main is clean.
  • check_no_names.py, with a local deny-list file: no match.

amd-server/rocm-gfx1151

🤖 Generated with Claude Code

glennneuber and others added 2 commits October 4, 2026 20:00
…t still needs a test

On 2026-10-04 the upstream CUDA maintainer said #27044 "looks 90% correct". He opened #29941, which pads the MMQ
ids path with ne12 instead of ne12*n_expert_used, and asked whether it works for us.

The submission record now holds the facts for the maintainer's reply:
- the confirmations, and #29847's test-backend-ops reproducer;
- how the two lines differ. From 512 tokens up they give the same padding, which covers every crash reported.
  Below that, #29941 can pad too little where the launch rounds J up;
- what this means for compat 903;
- the tests that are left for the CUDA host.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd the tests behind them

The CUDA host ran #445's checks against llama.cpp master 05043961 on an RTX PRO 6000 Blackwell (sm_120,
CUDA 13.0). It built three variants that differ only in the ids-path padding argument, and ran each case
under compute-sanitizer memcheck with the src1 buffer in its own exact-size allocation.

- #29941 (ne12) reads past its buffer at 65 and 100 tokens. #27044 (ne12*n_expert_used) is clean in
  every case.
- At 508 and 2040 tokens, #29847's cases and the original fault, both lines are clean. master fails
  everywhere.
- The stock pool hides the over-read in every short-batch case.
- master's mm_ids_helper fails to launch on this host, with 1 KB of static shared memory and a dynamic
  limit raised to the device maximum. A one-line workaround, common to all three builds, gets past it.

Adds the GPU-free check (tasks/mmq-ids-padding-test.cu); the llama.cpp patch with the six cases, the
debug switch and the workaround (tasks/mmq-ids-padding-gpu.patch); and the driver that builds the
variants and runs the matrix (tasks/mmq-ids-padding-gpu.sh).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@glennneuber

Copy link
Copy Markdown
Author

The CUDA host ran #445's tests on sm_120, at the maintainer's request. #29941 reads past its buffer below 128 tokens; #27044 is clean in every case. The record and the test code are in 7786fe093 on this branch.

Setup.

  • Hardware and llama.cpp: an RTX PRO 6000 Blackwell (sm_120), CUDA 13.0, driver 580.126.18, llama.cpp master 05043961.
  • Three builds of test-backend-ops, which differ only in the ids-path padding argument:
    • master: ne11;
    • #29941: ne12;
    • #27044, which is compat 903's line: ne12*n_expert_used.
  • One process per case, under compute-sanitizer --tool memcheck.

Memcheck errors when the src1 buffer has its own exact-size allocation (✓ test passed, ✗ aborted):

case master #29941 #27044
65 tokens, 576 rows (fallback), q4_K, 256 experts, 8 used 3,041 ✗ 3,229 ✗ 0 ✓
100 tokens, 512 rows, q4_K, 256 experts, 8 used 6,693 ✗ 557 ✗ 0 ✓
100 tokens, #29847's shape (q4_0, 512 experts, 10 used) 7,341 ✗ 53 ✗ 0 ✓
2040 tokens, the original fault's shape 101 ✗ 0 ✓ 0 ✓
508 tokens, #29847, b=0 913 ✗ 0 ✓ 0 ✓
508 tokens, #29847, b=1 2,397 ✗ 0 ✓ 0 ✓

With the stock memory pool, every cell is 0 ✓ except master on #29847's two cases (237 ✗ and 1,257 ✗).

What it shows.

  • #29941 fails where the tile is wider than its padding. That happens below 128 tokens.
    • At 65 tokens with 576 rows, the fallback configs launch J = 128. #29941 pads 64 blocks, and #27044 pads 128.
    • At 100 tokens, J = 112 against 96 and 128.
    • #29941's faulting reads come from mul_mat_q<q4_K, 128, fallback>, past an allocation of 1,207,296 bytes. That is the 1,198,080 data bytes plus its 64 blocks of 144 bytes.
  • At 508 and 2,040 tokens, both lines pad 128 blocks, so #29847's cases and our original fault do not separate them.
  • An error count does not measure the overrun. Memcheck stops each warp at its first out-of-bounds load, about 1 KB past every variant's allocation (also with --padding 65536). Two runs of the same #29941 build gave 2,337 and 3,361. Only zero against non-zero carries information.
  • The stock pool hides the over-read in every short-batch case. That is why the runs need the exact allocation.

Two local changes, common to all three builds:

  • MMQ445_EXACT=1 gives the src1 buffer an exact-size cudaMalloc. It is debug only.
  • A workaround for a launch failure. On master 05043961, every MoE MUL_MAT_ID through MMQ failed on this host with invalid argument at the mm_ids_helper launch, master's own test cases included. That happened before any padding code ran.
    • The helper has 1 KB of static shared memory, and CUDA_SET_SHARED_MEMORY_LIMIT raises its dynamic limit to the device maximum.
    • A standalone kernel with the same 1 KB array gets the same error.
    • The workaround keeps the default limit.
    • This is a separate upstream problem. It has not been checked on other GPUs or CUDA versions.

Test code, in 7786fe093:

  • docs/maxusai/tasks/mmq-ids-padding-test.cu, the CPU check. It needs nvcc, but no GPU.
    • It replays the padding rule against llama.cpp's own config functions, over 2,438,400 shapes.
    • #29941 leaves 267,960 of them short, by up to 63 blocks, all below 128 tokens. #27044 leaves none.
  • docs/maxusai/tasks/mmq-ids-padding-gpu.patch: the six test_mul_mat_id cases, the debug switch and the workaround. It applies cleanly to master 05043961.
  • docs/maxusai/tasks/mmq-ids-padding-gpu.sh <llama.cpp checkout>:
    • It applies the patch and builds the three variants.
    • It runs the whole matrix under memcheck.
    • It works on any NVIDIA GPU that takes the MMQ path.

The results section is in docs/maxusai/upstream-mmq-submission-material.md, "Results on sm_120 (2026-10-04)". Nothing was posted on #27044 or #29941. The reply there is the maintainer's to write.

ai-server/mlx-cuda

…pstream's

The record said the failure is a separate upstream problem. It is not shown to be. mmid.cu is
byte-identical in b11081, b11232 and master 05043961, and production's b11081 build runs this path on
the same card. Production's sm_120a PTX for the helper declares no static shared memory, while these
builds' native sm_120a code reports 1 KB. The cause more likely lies in these builds' configuration.
The padding results do not change, because the workaround is the same in all three builds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@glennneuber

Copy link
Copy Markdown
Author

A correction to my comment above: the mm_ids_helper launch failure is not shown to be an upstream problem.

  • mmid.cu has not changed. It is byte-identical in b11081, b11232 and master 05043961.
  • Production runs this path. Its b11081 build runs MoE MUL_MAT_ID through MMQ on the same card.
  • The 1 KB of static shared memory is in my builds only.
    • Production's sm_120a PTX for the helper declares none, and the GPU compiles it to SASS at load time.
    • My builds compiled native sm_120a code with CUDA 13.0, as a static build, and that code reports 1 KB.
  • So the failure more likely comes from my build configuration than from upstream's code.

The padding results do not change: the workaround was the same in all three builds. The record now says this, in e0c6b93d5.

ai-server/mlx-cuda

…heck starts at MMVQ's real limit

Three corrections found in review:

- On NVIDIA, no tile wider than 128 has a config, so both lines pad 128 blocks from 128 tokens up.
  #29941 pads less only below 128 tokens, not below 512.
- At 100 tokens on sm_120 there is no config at J = 104. The launch takes J = 112, so #29941 can fall
  up to 15 blocks short, not 7.
- The CPU check assumed MMVQ takes every MUL_MAT_ID batch up to 8 tokens. On Turing and newer it does
  not for q2_K (7) or q3_K (5), so 960 shapes were missing. The check now starts each type above its
  real limit. Corrected counts: 2,439,360 shapes, #29941 short in 268,440, #27044 in none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@glennneuber
glennneuber merged commit ab00f28 into main Oct 4, 2026
5 checks passed
@glennneuber

Copy link
Copy Markdown
Author

Review of #445 at 16c953b59, by the CUDA host at the maintainer's request: OK after three corrections, which are in 16c953b59. Merged as ab00f2840.

Corrected:

  1. "Below 512 tokens, #29941 pads less" should say 128. On NVIDIA, no tile wider than 128 has a config, so get_J_max() returns at most 128. From 128 tokens up, both lines pad 128 blocks. At 508 tokens both measured clean.
  2. At 100 tokens, the launched tile is J = 112, not 104. sm_120 has no config at 104. So #29941 can fall up to 15 blocks short, not 7, and the measurement agrees: 557 errors.
  3. My CPU check started every type at 9 tokens. It assumed MMVQ takes every MUL_MAT_ID batch up to 8. On Turing and newer it does not for q2_K (7 tokens) or q3_K (5), so 960 shapes were missing.
    • The check now starts each type above its real limit, replicated from get_mmvq_mmid_max_batch_turing_plus.
    • Corrected: 2,439,360 shapes. #29941 is short in 268,440, all below 128 tokens. #27044 is short in none.
    • The record also notes that with two or more experts per token, the MMQ path always has at least 12 rows.

Checked:

  • The published driver runs end to end. I ran mmq-ids-padding-gpu.sh on a copy limited to the 65-token case.
    • It built the three variants in a fresh build directory.
    • Exact allocation: master 3,021 errors ✗, #29941 2,521 ✗, #27044 0 ✓. Stock: all three 0 ✓.
    • It restored mmq.cu and left no process on the GPU.
  • The patch applies cleanly to llama.cpp master 05043961.
  • The CPU check compiles against that master and exits 0.
  • The table matches the run logs, cell for cell. The counts were re-parsed from the ERROR SUMMARY lines.
  • Housekeeping: check_source_paths.py --changed-since origin/main is clean, and the name scan is clean.

The #27044 reply stays the maintainer's to write.

ai-server/mlx-cuda

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.

1 participant