vulkan: raise the hoisted row-id limit for mul_mat_id from 256 to 512 experts - #28501
vulkan: raise the hoisted row-id limit for mul_mat_id from 256 to 512 experts#28501drluoto wants to merge 1 commit into
Conversation
|
Hi @drluoto, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
The expert-count shader (count_experts.comp) sizes its shared arrays with BLOCK_SIZE, which is 256. Because of that, row-id hoisting is switched off for any model with more than 256 experts, and every mul_mat_id workgroup has to rescan the whole ids tensor on its own. Qwen3.8-Flash-Next has 512 experts and was quietly running on that slow path. This change sizes the arrays with a separate MAX_EXPERTS constant (512), clears them in a loop instead of one entry per thread, and raises the matching limit on the host side. On Strix Halo at batch 2048 the expert matmuls drop from 12.5 to 9.5 ms (iq3_s) and from 14.0 to 7.5 ms (iq4_nl) per op, and prompt processing gets about 19 % faster at 8k tokens. test-backend-ops MUL_MAT_ID passes (891/891) with new 512-expert test cases. Assisted-by: Claude Fable 5.1
ec38ad3 to
266464c
Compare
|
I saw the rules on not using AI assistance besides the code, I can admit i am so used to always working AI-first. However I have personally reviewed and edited by hand everything above. I hope someone else may find use of some of the optimisations. |
|
I think this change is fine as-is, but should we increase it to 1024 to match LLAMA_MAX_EXPERTS? |
|
+1 |
… experts Upstream PR ggml-org#28501 (drluoto, draft). count_experts.comp sized its shared arrays with BLOCK_SIZE, so hoisting was off for models with more than 256 experts. Qwen3.8-Flash-Next has 512. Assisted-by: Hermes Agent
Overview
I was working on trying different optimizations for this stack together with Fable 5.1 in CC. We stumbled across this finding that
count_experts.compsizes its shared arrays withBLOCK_SIZE(256), soggml_vk_mul_mat_id_q_f16turns row-id hoisting (#26686) off when a model has more than 256 experts. Every mul_mat_id workgroup then rescans the whole ids tensor. Qwen3.8-Flash-Next has 512 experts.This fix seems to work:
The patch sizes the shared arrays with a separate
MAX_EXPERTS(512), clears them in a loop instead of one entry per thread, and raises the host limit to match.Measured on Strix Halo (Radeon 8060S, RADV, Mesa 26.0.3), 512 experts, 10 used, batch 2048, µs per op:
End to end this makes prompt processing faster. On Qwen3.8-Flash-Next Q5_K, prefill throughput (tokens per second, higher is better) goes from 426 to 507 t/s for an 8k prompt (+19 %) and from 333 to 387 t/s for a 32k prompt (+16 %). Token generation speed is unchanged, since it does not use this code path, and greedy output is identical before and after.
Additional information
Sidenote:
test-backend-ops -o MUL_MAT_IDpasses on master plus this patch (891/891), including the new 512-expert cases (n = 1, 5, 64, 300; iq3_s and q4_0). Found while profiling prefill on this box, see the discussion in #27950.This is one piece of a larger Strix Halo stack for Qwen3.8-Flash-Next. For the decode side, mainly MTP speculation with a vocabulary-trimmed draft head (FR-Spec), see my write-up in #27950, the comment on #25187, the branch https://github.com/drluoto/llama.cpp/tree/frspec-qwen4exp-strix and the draft heads at https://huggingface.co/drluoto/Qwen3.8-Flash-Next-MTP-GGUF.
Requirements