Skip to content

Fix non transposed affine qmm dispatch logic - #4392

Merged
zcbenz merged 1 commit into
ml-explore:mainfrom
RohanGautam:qmm_n_dispatch
Sep 7, 2026
Merged

zcbenz merged 1 commit into
ml-explore:mainfrom
RohanGautam:qmm_n_dispatch

Conversation

@RohanGautam

Copy link
Copy Markdown
Collaborator

The path for using the non-transposed NAX qmm kernel was unreachable in the current implementation (because of the condition if (has_nax_kernel && transpose && (K % 64 == 0) ... - lets only the transposed case through).

We needed to add some more conditions:

  • having N be a multiple of 64 (the non-transposed nax kernel does not have an unaligned-N variant)
  • Adding a transposed check for the block size since a non-transposed kernel with bm=32 is not instantiated. This check would not be needed if we instantiate it (adding to the metallib), but I didn't want to assume that that was an option. We can add it and simplify this condition if that's an option.

benchmarks

The benchmarks are run with mlx_lm linked to the different mlx versions. The non-transposed path is exercised for large prefills (when chunked), for the quantized kv cache computations. We use mlx_lm.generate (not benchmark, to use the kv cache quantisation options) . The command run is mlx_lm.generate --model <model> -p - --ignore-chat-template --max-tokens 8 --temp 0 --kv-bits 4 --quantized-kv-start 0 < <file withprompt of desired size>.txt

model machine prompt prefill before prefill after Δ prefill
Qwen3-8B-4bit M5 Pro 4096 1,424.7 1,425.6 +0.06%
6144 1,231.3 1,290.4 +4.80%
8192 1,092.4 1,182.4 +8.24%
12288 907.2 1,028.1 +13.33%
M5 Max 4096 2,791.3 2,791.1 −0.01%
6144 2,233.7 2,277.5 +1.96%
8192 1,969.8 2,111.2 +7.18%
12288 1,647.2 1,819.6 +10.47%
Qwen3-4B-Instruct-2507-4bit M5 Pro 4096 2,127.9 2,127.4 −0.02%
6144 1,716.5 1,834.7 +6.89%
8192 1,466.5 1,630.9 +11.21%
12288 1,146.6 1,344.2 +17.24%
Qwen3-30B-A3B-4bit M5 Max 4096 3,384.4 3,399.7 +0.45%
6144 2,721.2 2,906.6 +6.81%
8192 2,289.7 2,539.8 +10.92%
12288 1,690.6 1,967.7 +16.39%

The 4096 one has no changes because of mlx-lm using N-1 tokens instead of N tokens. There is only one quantised kv cache to attend over, and this is the wrong size for the non-transposed qmm to kick in (it does not have a ragged-K variant). The last chunk for all others also go through the normal path, just the middle ones don't and are sped up with this PR. This boundary effect can be seen below, and is perhaps something to address in mlx-lm down the line.

model machine prompt prefill before prefill after Δ prefill
Qwen3-8B-4bit M5 Pro 4096 1,428.4 1,428.6 +0.01%
4097 1,452.0 1,580.3 +8.83%
6144 1,224.2 1,280.4 +4.60%
6145 1,246.0 1,405.9 +12.83%
M5 Max 4096 2,789.5 2,791.3 +0.07%
4097 2,749.4 2,894.9 +5.29%
6144 2,101.4 2,212.7 +5.30%
6145 2,237.0 2,478.7 +10.80%
  • ☑️ I understand it is strictly prohibited to use AI to write PR description
  • AI usage disclosure: AI was used to flag candidate optimisations, which I then understood, ran benchmarks for, and simplified.

@RohanGautam
RohanGautam marked this pull request as ready for review August 25, 2026 07:39
@nastya236
nastya236 requested a review from jagrit06 August 25, 2026 09:40
@nastya236

nastya236 commented Aug 25, 2026 •

Copy link
Copy Markdown
Collaborator

Looks good to me! We probably have the same issue in gather_qmm..

Regarding mlx-lm, ml-explore/mlx-lm#1781 proposed to add a prefill size as an argument, so it is more controllable now!

@nastya236
nastya236 self-requested a review August 25, 2026 22:01

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were a few contributors looked into this:
#4051
#4202

My main concern was that I don't know why it was not enabled when there was apparently not much work to get it working so I was worried about missing something, can you double check with @jagrit06?

@RohanGautam

Copy link
Copy Markdown
Collaborator Author

There were a few contributors looked into this: #4051 #4202

My main concern was that I don't know why it was not enabled when there was apparently not much work to get it working so I was worried about missing something, can you double check with @jagrit06?

Yep! I wonder if I'm missing something too. Now that you pointed out, this was a fix for a bug introduced in #4202 which the author for #4051 mentioned as a comment on that PR.

Also now that i look into this more, the qmm_n_nax kernels for non-affine quantisations aren't instantiated, which is why the =="affine" condition was needed. @zcbenz You mentioned they are still a WIP - is there a PR for this? I could keep track and simplify that condition once it lands. Also happy to look into those kernels if needed

@zcbenz

zcbenz commented Aug 26, 2026

Copy link
Copy Markdown
Member

@RohanGautam There is no active works on NAX kernels, the main reason I was hesitated about enabling it was because I was not sure if I would get time to look into issues if anything goes wrong, if you are confident to look into possible followup issues, then let's merge.

@zcbenz
zcbenz merged commit 3854df9 into ml-explore:main Sep 7, 2026
29 checks passed
aaishwarymishra pushed a commit to aaishwarymishra/mlx that referenced this pull request Sep 11, 2026
nicholasjpaterno added a commit to Epistates/pmetal that referenced this pull request Sep 16, 2026
…sses

v0.32.2 (2026-08-25) is the newest tag, and it predates several correctness
fixes in the quantized matmul path the Gemma 4 packed-weight work builds on:
the non-transposed affine qmm dispatch (ml-explore/mlx#4392), fp qmm
corruption when the quantized dimension is not a multiple of 32 (#3912), and
two sorted `gather_qmm` fixes (#4009, #3922). It also predates D512 vector
attention (#4459) and the fused NAX head-dim-256 prefill path (#4416), which
cover Gemma 4's full-attention and sliding layers respectively.

`gather_qmm` gained a `global_scale` parameter between the mode and
`sorted_indices`, so the positional call bound a bool to
`std::optional<array>` and stopped compiling. That argument and
`sorted_indices` are named now, since the next insertion would otherwise bind
silently.

No speedup, and none was available: decode on a 12B is memory-bandwidth
bound. 51.8 ms/token measured against a 44.0 ms roofline for 24 GB of bf16
weights on an M4 Max is 85% of theoretical, so no kernel change can move it.
Reading 6.3 GB of packed 4-bit instead is the only lever, which is #27.

Verified: full preflight, plus Llama-3.2-1B, Qwen2.5-0.5B, gemma-2-2b and
Phi-3-mini at 4-bit and gemma-4-12B at bf16 and 4-bit all still decode.

Refs #27
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.

3 participants