vulkan: restore turbo_wht op + turbo3/4 FA dispatch (regression from b9190 upstream sync)#160
Merged
Conversation
…b9190 upstream sync) The b9190 upstream-sync merge (e30bbcf) clobbered the Vulkan turbo KV cache support that landed in PR #62 (ff8bb73). Shaders survived, but the C++ dispatch path in ggml-vulkan.cpp was dropped during merge conflict resolution. This restores the lost wiring, adapts it to the post-sync Vulkan API, and adds two fixes that surface on RDNA4. Closes #88, #89. Tracking: #159. Restored / added: - pipeline_turbo_wht member + creation, ggml_vk_turbo_wht dispatch fn, GGML_OP_TURBO_WHT op handler. Reuses existing turbo_wht.comp shader. Explicit ggml_vk_sync_buffers barriers around dispatch for compute-to-compute RAW/WAW ordering. - CREATE_FA(GGML_TYPE_TURBO3_0, ...) SPIR-V for scalar + cm1 coopmat FA paths (cm2 NV-coopmat skipped, fp32-only for turbo3). - fa_kv_ok extended to accept turbo2/3/4 in supports_op(FA). - flash_attn_dequant.glsl: turbo K/V SSBO bindings with restrict + std430 to avoid alias-stride issues, FA_DEQUANT4_TURBO{2,3,4}_0 macros for per-element centroid lookup (no iWHT in kernel — graph applies WHT to Q pre-attention and inverse to output post-attn). - Dedicated DATA_A_TURBO3_0 SPIR-V variant gates away the generic f16/q4/q5/q8 binding aliases — eliminates SSBO alias collision on RDNA where mismatched stride aliases at the same binding caused driver mis-strided loads. - memoryBarrierShared() added in flash_attn_cm1.comp at shared-mem load sites for kvsh to fix the cm1 coopmat read-before-write race. - flash_attn.comp + flash_attn_cm1.comp gate f16 K/V aliases under #if !defined(DATA_A_TURBO3_0). - fa_block_elems extended with turbo2/3/4 cases (types 42/43/44). - SET_ROWS pipelines registered for turbo2/turbo3/turbo4, with the required head_dim%128 guard in supports_op. - op_f32 dispatch math special-cases turbo: 128 threads per WG handling one full QK=128 block, so ne = ne/128 rather than the generic CEIL_DIV(ne, 32 * blck_size). Two root causes (out of ~8 hypotheses): 1. requiredSubgroupSize=32 on turbo set_rows pipelines. RDNA4 driver picks wave64 by default for compute pipelines unless explicitly constrained. The turbo set_rows shader uses subgroupBallot to pack 32 sign bits into a single uvec4.x word; on wave64 the ballot covered 64 lanes and the sign packing was off by 32, silently producing garbage in cache writes. Pinning to wave32 via VkPipelineShaderStageRequiredSubgroupSizeCreateInfo makes the ballot semantics deterministic. This is THE root cause of #88's incoherent decode on RDNA4. 2. ne = ne/128 dispatch math. turbo set_rows runs one full QK=128 block per workgroup with 128 threads. Generic quant path uses 32 threads per WG with CEIL_DIV(ne, 32 * blck_size). Without the special case, the turbo path was under-dispatched by 32x, leaving most of the cache uninitialized. This was the missing line from the original April implementation in ff8bb73 — present then, dropped during b9190 sync. Verification on RX 9070 XT (RDNA4, gfx1201), Qwen2.5-7B Q4_K_M: Config pp512 tg128 Status --cache-type-k f16 --cache-type-v f16 2636 t/s 48.4 no regression --cache-type-k q8_0 --cache-type-v turbo4 1951 t/s 49.2 coherent --cache-type-k q8_0 --cache-type-v turbo3 446 t/s 45.6 coherent --cache-type-k q8_0 --cache-type-v turbo2 453 t/s 45.2 coherent All decode against HIP turbo3 baseline produces coherent output. Prefill on Vulkan now lands at ~3.3x the previously-measured HIP throughput on the same hardware. Decode is ~56% of HIP; tuning headroom for follow-up. #137 (RDNA 3.5 GFX1151 case) not retested — requires Strix Halo hardware. Likely fixed by the same requiredSubgroupSize=32 fix since RDNA 3.5 has the same wave-flexible scheduling, but unverified. Co-Authored-By: Simon Gardling <titaniumtown@proton.me> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@Titaniumtown — heads up, this PR restores most of your PR #62 ( |
value-added-korea
pushed a commit
to Value-Added-International/llama-cpp-turboquant
that referenced
this pull request
Jun 6, 2026
vulkan: restore turbo_wht op + turbo3/4 FA dispatch (regression from b9190 upstream sync)
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.
Summary
Restores Vulkan turbo3/4 KV cache support that was clobbered during the b9190 upstream-sync merge (
e30bbcfe5). Brings back the wiring originally landed in PR #62 (ff8bb7394), adapts it to the post-sync Vulkan API, and adds turbo2/turbo4 FA dispatch + therequiredSubgroupSize=32constraint that's required on RDNA4.Tracking: #159 (regression discovery). Closes: #88, #89.
What was broken on current main
pipeline_turbo_whtmember,ggml_vk_turbo_whtdispatch fn, andGGML_OP_TURBO_WHTcase handler all missing — graph-level WHT rotation for Q (and inverse rotation post-FA) silently fell back or failedCREATE_FA(GGML_TYPE_TURBO3_0, ...)SPIR-V variants not built — FA dispatch had no turbo pipeline to findfa_kv_okrejected turbo types insupports_op(FLASH_ATTN_EXT)flash_attn_dequant.glslhad no turbo cases indequantize4()Net effect:
--cache-type-k turbo3 --cache-type-v turbo3on Vulkan failed at startup, and asymmetric configs produced incoherent output.Fix
Three commits:
1. SET_ROWS pipeline registration for turbo2/3/4
vulkan-shaders-gen.cppset_rows generation loopset_rows_turbo*_0_{i32,i64}pipelines inggml-vulkan.cpphead_dim % 128 == 0guard insupports_op(SET_ROWS)matching the 128-element block constraintne = ne / 128rather than the genericCEIL_DIV(ne, 32 * blck_size)requiredSubgroupSize = 32on the three turbo set_rows pipelines. Without this the RDNA4 driver picked wave64 by default, breaking thesubgroupBallot.x32-lane sign-packing inside the turbo set_rows shader. This is THE root cause of the incoherent decode in Eval bug: Vulkan turbo3 KV produces incoherent decode while HIP on same model is fine (7900 XTX, head_dim=128) #882.
ggml_turbo_whtVulkan oppipeline_turbo_whtmember + creation reusing the existingturbo_wht.compshaderggml_vk_turbo_whtdispatch function (readsdirection,group_sizefromop_params, dispatches with explicitggml_vk_sync_buffersbarriers around for compute-to-compute RAW/WAW ordering)GGML_OP_TURBO_WHTcase in the build-graph dispatch switchsupports_op(GGML_OP_TURBO_WHT)returns true for F32 src withne[0] % 128 == 03. Flash Attention dispatch for turbo3 K/V
fa_kv_okcases for turbo2/3/4 insupports_op(FLASH_ATTN_EXT)flash_attn_dequant.glsl(restrict std430to avoid alias-stride issues on RDNA when the same binding has mismatched-stride aliases)FA_DEQUANT4_TURBO{2,3,4}_0macros: per-element centroid lookup, no iWHT in the kernel because the graph applies WHT to Q pre-attention and inverse WHT to output post-attention (orthogonality:Q · K_rot = Q_rot · K_rot)fa_block_elemscases for turbo2/3/4 →QUANT_K_TURBO*_0DATA_A_TURBO3_0SPIR-V variant (scalar + cm1 coopmat paths) that gates away the generic f16/q4/q5/q8 binding aliases — eliminates the SSBO alias collision on RDNA where mismatched stride aliases at the same binding caused mis-strided loadsmemoryBarrierShared()added inflash_attn_cm1.compat the shared-memory load sites for kvsh to fix the cm1 coopmat read-before-write raceflash_attn.compandflash_attn_cm1.compgate the f16 K/V aliases under#if !defined(DATA_A_TURBO3_0)so the turbo3 variant only has turbo bindings at slots 1/2Note: cm2 (NV cooperative matrix v2) FA path is intentionally skipped in this PR — turbo3 is f32-accumulator only. NV users get the cm1 KHR-coopmat path. cm2 specialization can be a follow-up.
Two real root causes (out of ~8 hypotheses)
requiredSubgroupSize = 32on turbo set_rows — RDNA4 driver picks wave64 by default for compute pipelines unless explicitly constrained. The turbo set_rows shader usessubgroupBallotto pack 32 sign bits into a single uvec4.x word; on wave64 the ballot covered 64 lanes and the sign packing was off by 32. This silently produced garbage in the cache writes. Pinning to wave32 viaVkPipelineShaderStageRequiredSubgroupSizeCreateInfomakes the ballot semantics deterministic.ne = ne / 128dispatch math — turbo set_rows runs one full QK=128 block per workgroup with 128 threads. The generic quant path uses 32 threads per WG withCEIL_DIV(ne, 32 * blck_size). Without the special case, the turbo path was under-dispatched by 32×, leaving most of the cache uninitialized.The second was the missing piece from the original April implementation — present in
ff8bb7394, dropped during the b9190 sync. The first is RDNA4-new: didn't matter on the hardware the April PR was developed on, surfaces on wave-flexible hardware.Verification
All on RX 9070 XT (RDNA4, gfx1201), Qwen2.5-7B Q4_K_M, current branch HEAD:
--cache-type-k f16 --cache-type-v f16(baseline)--cache-type-k q8_0 --cache-type-v turbo4(recommended)--cache-type-k q8_0 --cache-type-v turbo3--cache-type-k q8_0 --cache-type-v turbo2All decode against HIP turbo3 baseline produces coherent output. Prefill on Vulkan now lands at ~3.3× the previously-measured HIP throughput on the same hardware. Decode is ~56% of HIP; there's headroom for follow-up tuning (workgroup-size sweep, possibly turbo3-specific FA pipeline tuning).
#137 (RDNA 3.5 GFX1151 case) not retested here — requires Strix Halo hardware. Likely fixed by the same
requiredSubgroupSize=32mechanism since RDNA 3.5 has the same wave-flexible scheduling, but unverified.Files touched
ggml/src/ggml-vulkan/ggml-vulkan.cpp(host-side wiring)ggml/src/ggml-vulkan/vulkan-shaders/flash_attn.comp(turbo3 binding guards)ggml/src/ggml-vulkan/vulkan-shaders/flash_attn_base.glsl(fa_block_elems cases)ggml/src/ggml-vulkan/vulkan-shaders/flash_attn_cm1.comp(turbo3 binding guards + memoryBarrierShared)ggml/src/ggml-vulkan/vulkan-shaders/flash_attn_dequant.glsl(turbo K/V bindings + dequant macros + DATA_A_TURBO3_0 specialization)ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp(turbo3 FA SPIR-V variants + set_rows turbo + turbo_wht)212 insertions / 4 deletions across 6 files.
Follow-ups
vulkan: turbo3/4 decode perf vs HIP gap (~44% headroom on RX 9070 XT)— workgroup-size sweep, possibly cm2 pathvulkan: cm2 (NV cooperative matrix v2) path for turbo3 FA— NV users currently on cm1, optimal would be cm2vulkan: extend DATA_A_TURBO2_0 / DATA_A_TURBO4_0 SPIR-V specialization— turbo2/4 currently use the generic FA SPIR-V which has all binding aliases declared; if RDNA alias issues surface there, dedicated variants would be the fix