Extending commit for PR#4077 for GQA size 12 and 16 - #4380
Open
dudududukim wants to merge 2 commits into
Open
Conversation
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.
Following PR for #4077 (extending group size of 12 and 16 w\ head_dim=128)
As described in #4077 reading KV Cache has redundancy wrt Query head number.
While former PR solved GQA (gqa factor=8), this PR further expand to gqa factor=12or16.
All measurement was taken with Power-connected with power mode 2(performance mode) with M5 pro 24GB macbook pro.
Reference-kernel deficit, re-measured (32K, hd128, H_kv=4, unique KV 67.1MB, measured peak 265 GB/s):
About Heads Per Threadgroup(HPT) values
HPT is bounded by threadgroup memory (hard 32KB).
The staging buffer is
G * HPT * Vfloats:Registers bound it from the other side: HPT=8 spills at hd128 (established in the #4077 work), so HPT <= 4. HPT=1 makes NT=G, which is mathematically identical to the reference kernel, so gqa 16 has no third option here.
Further improvement is planned to be added with follow-up PR.
Target models
As we could not run full model with 24GB memory, we just used config.json to run the same compute operation rather than loading the weight. (It would be great to be verified by other resource sufficient user! @erwinzhang7)
config.json verified, all head_dim=128:
Kernel benchmarks
Following the
CONTRIBUTING.mdtest script was tested both on main and PR branch with everything un-changed.(a) Two-build comparison —
origin/mainbuild vs PR build, viabench_sdpa_gqa.py:fp16, hd128, H_kv=4. Median of 9 trials x 16 iterations.
mainis 0.32.2.dev20260823+7789905ed,PRis 0.32.2.dev20260823+b7270a558:gqa 4 is outside the gate on both builds. gqa 8 is inside the gate but runs byte-identical code on both builds, which makes it the stronger control: it isolates cross-build drift from the effect, and lands at 1.00-1.02x.
(b) Speedup measured :
sdpa_vector_2pass_1(reference) vssdpa_vector_2pass_1_gqa_N(new kernel)Truncated end-to-end speedup
Models using gqa 12/16 cannot be fit in our 24GB, same as #4077, we tested with keeping the real per-layer model structure and truncating some last layers.
(reduced the expert pool from 128 to 16 while keeping num_experts_per_tok at 8)
GLM-4.5-Air (gqa 12), 16 of 46 layers:
Qwen3-235B (gqa 16), 16 of 94 layers: 32K 1.031x, 64K 1.033x, 128K 1.061x
Truncation deflates the result, so these are floors: halving depth halves attention and MLP together while the per-step lm_head tail is fixed, making a shallow model less attention-dominated. Measured directly — GLM @32k: 4L 1.053x -> 8L 1.055x -> 16L 1.063x.
Extrapolation to full depth (Expected speed-up)
Regressing the depth sweep gives
step(L) = L x block + tail, so the fixed lm_head/embedding tail can be post-computed expanding the expected latency to full depth.GLM-4.5-Air — block 1.754 ms (main) / 1.642 ms (PR), tail 1.36 ms; the tail is ~2% of a 46-layer step, so truncation costs at most +0.2pp:
Qwen3-235B — block 1.256 ms (main) / 1.216 ms (PR), same 1.36 ms tail:
Both models recompute the same final part. Since they have the same ~152k vocabulary size and 4096 hidden size, they use the same
lm_head.The numbers above are linearly extrapolated from measurements on this machine. They are not performance predictions for hardware that can fit the entire model in memory.
Functionality check
vs fp32 naive attention, ctx 8k:
Gate execution was checked with Xcode GPU trace:
sdpa_vector_2pass_1_gqa_12_float16_t_128_128dispatched.Modified Test script
One line, extending the list #4077 introduced in
test_sdpa_vector_gqa_long:(48, 4, 128)is gqa 12,(64, 4, 128)is gqa 16, both hd128, both inside the gate at the existingL in [8192, 8201]. Those two L values give an exact split and a remainder split: at L=8201 with blocks=256 the chunk is 33tokens, so gqa 16's HPT=2 sub-chunk is 17 with the last one partial.
test_fast_sdpa23/23 pass. Sensitivity: measured gqa 16 fp32 maxdiff against fp32 naive is 5.59e-08 versus the test'satol=1e-4, ~1800x margin.Honest note on "confirm the new tests fail in the main branch": a correctness test for these shapes PASSES on main, because main falls back to the reference kernel and produces correct output. That half of the rule cannot be satisfied by a routing change with identical numerics. The test guards the new path; it does not reproduce a bug.
Formatting
uvx pre-commit run --all✅ PASSED