Skip to content

vulkan: fix FA dequant path engagement - #28190

Merged
0cc4m merged 1 commit into
ggml-org:masterfrom
Nathanw1014:fix/vk-fa-dequant-guard-ne3
Sep 3, 2026
Merged

vulkan: fix FA dequant path engagement#28190
0cc4m merged 1 commit into
ggml-org:masterfrom
Nathanw1014:fix/vk-fa-dequant-guard-ne3

Conversation

@Nathanw1014

Copy link
Copy Markdown
Contributor

Overview

Fixes #28135

Skip the nb[3] check when ne[3] == 1, the shader never reads it for a single stream. Cache views carry the full-buffer stride there, so the old check reduced to n_kv == kv_size and the path only engaged with the cache full.

Additional information

The #25494 dequant shader is a plain copy with no layout validation, the host
check has to prove the layout. It was too tight on dim 3: cache views carry the
full-buffer stride in nb[3], so it only accepted n_kv == kv_size.

+29% llama-server prefill on a 30B MoE with q8_0 KV, output byte-identical.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure:
    YES. Investigation and testing assisted by Claude (Opus 5).

Skip the nb[3] check when ne[3] == 1, the shader never reads it for a
single stream. Cache views carry the full-buffer stride there, so the old
check reduced to n_kv == kv_size and the path only engaged with the
cache full.
@Nathanw1014
Nathanw1014 requested a review from a team as a code owner September 1, 2026 16:58
@github-actions github-actions Bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Sep 1, 2026
@0cc4m
0cc4m merged commit c7bda03 into ggml-org:master Sep 3, 2026
35 of 43 checks passed
fewtarius pushed a commit to fewtarius/CachyLLama that referenced this pull request Sep 5, 2026
Skip the nb[3] check when ne[3] == 1, the shader never reads it for a
single stream. Cache views carry the full-buffer stride there, so the old
check reduced to n_kv == kv_size and the path only engaged with the
cache full.
wdenejko added a commit to wdenejko/llama.cpp that referenced this pull request Sep 6, 2026
Surgical sync: every conflict resolved hunk-by-hunk with our validated
implementation as the base, upstream taken where it is additive or a fix.

Conflicts and how they were resolved:
- ggml-vulkan.cpp: kept both sets of context members (our MM_TILE_EPILOG
  state + upstream's fused_topk_qsa), kept our GGML_VK_MMV_RM_* overrides and
  took upstream's Strix Halo mat-vec row tuning (ggml-org#27909, rm_int_n/rm_id),
  kept OUR FA dequant-once gate (upstream's is_dense_kv_cache lambda dropped;
  ggml-org#28190's ne[3]==1 semantics are already in our gate), kept both fusion
  resets.
- qwen4exp.cpp: kept our hc up_perm key, took upstream's hc_count/low_rank
  validation; kept our pooled-cache indexer path; adopted the new
  build_attn_mha(n_kv_max) signature with n_kv_max = 0 (sparse FA is
  CUDA/Metal-only and upstream keeps it off for qwen4exp); took upstream's
  recurrent-state conv snapshot-plane loop (ggml-org#28123) which is provably the
  same set of (slot, source window) writes as ours; hparams.n_ff_exp() at
  our MTP-layer loader.
- llama-memory-hybrid-idx.{cpp,h}: kept ours entirely. ggml-org#27941's fixes
  (multi-sequence block keying, 2D-mrope ranking, seq_cp, -kvu) are outside
  the served envelope (parallel 1, text only, no unified KV); its kv-cache
  ext restore part merged cleanly and is kept.
- llama-model-loader.cpp: upstream's lazy_read API with OUR semantics kept:
  lazy (mapping-resident) reads only under --load-mode mmap, model-wide
  no-prefetch when any tensor is lazy, zero-copy only for tensors bound to an
  mmap buffer, device tensors via the async upload + drop_page_cache,
  populate() of the lazy ranges after the uploads; upstream's mlock
  exclusion for lazy tensors and the RAM-peak staging order are adopted.
- llama-graph.cpp: the chunked recursive build_attn_mha call forwards
  n_kv_max.
- llama-arch.cpp: upstream added QWEN4EXP to the unconditional rollback list
  (duplicate case); our env-gated Q4X_RS_ROLLBACK case is kept.
- .github/workflows: the 17 upstream-modified workflows stay deleted.

Reviewed (merged cleanly, no interaction found): set_n_kv_max writes FA
op_params[4] which we never use; kv-cells seq_pos set + seq_pos_tok_le
(get_prev_tokens O(n_kv) scan gone); radix top-k engages only at k >= 1024
(served block-level top-k runs at ~512); n_expert_used/n_ff_exp per-layer
accessors; state restore chunking fix; n_layer_nextn read before use.

Compile-gated locally (macOS Vulkan build: llama-server, test-backend-ops,
llama-bench). Runtime gates (greedy identity, repeat determinism, perf A/B)
follow on the Strix Halo box before this reaches master.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
thecodacus pushed a commit to thecodacus/llama.cpp that referenced this pull request Sep 7, 2026
Skip the nb[3] check when ne[3] == 1, the shader never reads it for a
single stream. Cache views carry the full-buffer stride there, so the old
check reduced to n_kv == kv_size and the path only engaged with the
cache full.
SteelPh0enix pushed a commit to SteelPh0enix/llama.cpp-qwen4exp that referenced this pull request Sep 8, 2026
Skip the nb[3] check when ne[3] == 1, the shader never reads it for a
single stream. Cache views carry the full-buffer stride there, so the old
check reduced to n_kv == kv_size and the path only engaged with the
cache full.

(cherry picked from commit c7bda03)
SteelPh0enix pushed a commit to SteelPh0enix/llama.cpp-qwen4exp that referenced this pull request Sep 8, 2026
Skip the nb[3] check when ne[3] == 1, the shader never reads it for a
single stream. Cache views carry the full-buffer stride there, so the old
check reduced to n_kv == kv_size and the path only engaged with the
cache full.

(cherry picked from commit c7bda03)
x1250 pushed a commit to x1250/llama.cpp that referenced this pull request Sep 9, 2026
Skip the nb[3] check when ne[3] == 1, the shader never reads it for a
single stream. Cache views carry the full-buffer stride there, so the old
check reduced to n_kv == kv_size and the path only engaged with the
cache full.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Vulkan] FA f16-scratch fast path never enabled for hybrid-model (non-unified) KV caches

3 participants