perf(metal): E=256 expert-tile route for Qwen 3.5/3.6 MoE prefill + trust mode (unit 1.22-1.32x, e2e +15.2% @8k) - #6
Closed
Gajesh2007 wants to merge 3 commits into
Conversation
… prefill - templatize the sorted expert-tile descriptor builder on expert count; instantiate NE=128 (keeps the Gemma4 host name) and NE=256 (build_sorted_expert_tiles_bm32_e256). Tile kernel unchanged/shared. - classify_gemma4_expert_qmm: admit E=256 with Qwen geometries (fused gate_up [256,1024,2048], split gate/up [256,512,2048], down [256,2048,512]); geometry table tied to expert count. - try_gemma4_expert_qmm: expert count from w.shape(0); per-E descriptor kernel + dispatch; documented descriptor upper bound; TODO with evidence on the retract-only GPU->CPU sync (grid already over-dispatched). - device.cpp: AOT gate requires both builders + tile kernel, fail-closed. - tests: Qwen route-table coverage incl. E/geometry cross-rejection.
…ze mid-eval gpu::eval captured the MTL::CommandBuffer* BEFORE eval_gpu, but a primitive that calls CommandEncoder::synchronize() inside eval_gpu (the expert-tile route's sortedness-retract check) commits and REPLACES the encoder's buffer; eval then attached its buffer-liveness completion handler to the dangling pointer (objc_msgSend on freed object). The Gemma E=128 route has been surviving this by allocator luck; the Qwen E=256 shapes crash deterministically. Fetch the buffer from the encoder AFTER eval_gpu — the current buffer holds the tail of the primitive's work, which is exactly what the inputs must outlive.
…dback The tile grid is already over-dispatched; the mid-eval synchronize exists only to observe a retracted descriptor build. Under trust the caller asserts machine-guaranteed sorted indices (the Swift SwitchGLU prefill path sorts on-device); a genuine violation yields undefined output for that matmul instead of the legacy fallback. Measured: the drain cancels the tile kernel's 12-23% unit win end-to-end; trust recovers it (8k prefill 1364 -> 1433 tok/s).
This was referenced Aug 13, 2026
Member
Author
|
Closing in favor of #7: consolidating on darkbloom-base as the single integration line (repo convention — #1/#3/#4 all merged there, and #4 already folded codex/gemma4-autoresearch-v0.8.2 into it). #7 carries commit-for-commit identical tile work PLUS the gpu::eval UAF fix on darkbloom-base. mlx-swift consumers should repin Cmlx to the darkbloom-base lineage when #7 merges (tracked in mlx-swift#12). |
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.
Problem
Qwen3.6-35B-A3B MoE prefill routes its expert QMMs through the legacy
gather_qmmpath: E=256 experts with real multinomial top-8 routing produce expert row-boundaries that almost never align to 16-row tiles, so the legacy kernel pays tile inflation on every routed matmul (the exact diagnosis already fixed for Gemma E=128 by the expert-tile route — which is hard-wired to E=128 and rejects Qwen's expert count and geometries).On top of that, the route's sortedness-retract fail-safe reads a GPU counter on the host: one full stream drain per routed gather = 120 drains per 512-token chunk (3 gathers × 40 MoE layers), which erases the kernel win end-to-end.
Change
template <int NE> build_sorted_expert_tiles_bm32, quantized.h): NE=128 keeps the Gemma4 host name; NE=256 instantiatesbuild_sorted_expert_tiles_bm32_e256(quantized.metal). The tile kernel itself is unchanged and shared.classify_gemma4_expert_qmm(gemma4_expert_qmm.h): fused gate_up[256,1024,2048], split gate/up[256,512,2048], down[256,2048,512]; geometry table tied to expert count with cross-rejection both directions.try_gemma4_expert_qmm(quantized.cpp), with a documentedmax_tile_countupper-bound proof.MLX_GATHER_QMM_EXPERT_SLICES=trust(device.cpp/h, quantized.cpp): skips only the retract-counter host readback — the grid is already over-dispatched with device early-exit, so trust changes no dispatch. Under trust, a genuinely mis-sorted sorted-flag call yields undefined output for that matmul instead of legacy fallback; the only sorted-flag producer (mlx-swift-lm SwitchGLU) sorts on-device immediately before the call. TODO(1.3) documents the device-side fallback that would make trust unconditional.gpu::evalUAF fix (ab5e1dd) — required: the retract check synchronizes mid-eval, and without the fix the E=256 shapes segfault deterministically. Cherry-picked separately tomainas fix(metal): use-after-free in gpu::eval for primitives that synchronize mid-eval #5 (prod-relevant, should merge first).Measured (M4 Max 40c, release, prod 20 GB Qwen3.6 artifact)
Unit microbench (isolated gatherQuantizedMM, sorted, realistic multinomial top-8 histogram over 256 experts, T=512/M=4096, median of 25):
MoE routed block T=512 (shipping split-legacy 4.741 ms → fused+tiles+trust 3.492 ms) = −26.3%. End-to-end prefill (with the mlx-swift-lm fuseGateUp PR): 8k prompts 1243 → 1433 tok/s (+15.2%), 2k +7.4%, 32k +6.6%. With the retract drain left on, the unit win nets ≈0 e2e (1364 vs 1364 @8k) — that is what
trustrecovers. Uniform tile-aligned histograms flip the unit result (legacy wins ~15%), confirming the tile-inflation diagnosis: the win comes exactly from expert boundaries not aligning to 16-row tiles.Behavior
flowchart LR subgraph Before A1["Qwen E=256 routed QMM"] --> B1["route rejects E!=128 → legacy gather_qmm<br/>16-row tile inflation on every expert boundary"] --> C1["+ retract fail-safe = host drain ×120/chunk<br/>8k prefill 1243 tok/s"] end subgraph After A2["Qwen E=256 routed QMM"] --> B2["E=256 descriptor builder → shared tile kernel<br/>unit 1.22–1.32x"] --> C2["trust skips retract readback only<br/>8k prefill 1433 tok/s (+15.2%)"] endCode
flowchart LR subgraph Before D1["quantized.h: build_gemma4_sorted_expert_tiles_bm32 (E=128 hard-wired)"] --> E1["gemma4_expert_qmm.h: classify rejects E=256"] --> F1["quantized.cpp: retract sync always drains"] end subgraph After D2["quantized.h: template<int NE> builder — NE=128 keeps Gemma name, NE=256 new symbol"] --> E2["gemma4_expert_qmm.h: E∈{128,256} topology + per-E geometry table, cross-rejected"] --> F2["quantized.cpp: per-E descriptor select + tile-count bound; trust skips readback"] F2 --> G2["device.cpp: env incl. trust; 3-symbol AOT fail-closed gate"] endTests
tests/gpu_tests.cpp"Qwen 3.6 expert QMM pure route table": hits ×3 geometries ×{4096,8192,16384}; topology/geometry/assignment/quantization/metallib misses; E×geometry cross-rejection both directions.SortedGatherQuantizedMMTests, route on): 10/10 — Gemma suite intact; Qwen closed-form fixtures all 3 geometries at M∈{4096,8192,16384} incl. empty experts, unaligned multi-boundary tiles, 255×1-row fragmentation; random-tensor equivalence vs legacy and vs dequantized gatherMM; sortedness-violation retract→legacy with correct output; T=128 legacy fallback by design.Stack
Merge order (top to bottom):
gpu::evalUAF fix (basemain) — prod-relevant crash fix, merge firstcodex/gemma4-autoresearch-v0.8.2, the line mlx-swift pinsSource/Cmlx/mlxto) · perf(metal): E=256 expert-tile route + trust + gpu::eval UAF fix — darkbloom-base mirror #7 — same commits mirrored ontodarkbloom-base(the line d-inference pinslibs/mlxto) ← this PRmain)perf(cbv2-mtp): Qwen3.6 GDN capture-verify + target-prefix acceptance (1.53x vs serial verify) mlx-swift-lm#106— GDN capture-verify — merged (squashd06da67)main)master)perf/qwen36-mtp-capture-verify= Fix typo in CMakeLists.txt ml-explore/mlx#616's head)Chain: mlx UAF fix → mlx tiles (+mirror) → mlx-swift →
mlx-swift-lm#106(merged) → mlx-swift-lm prefill → d-inference#616 → d-inference prefill.