vulkan: fix wrong results when a mul_mat reads a slice of a larger cache by ServeurpersoCom · Pull Request #28956 · ggml-org/llama.cpp

ServeurpersoCom · 2026-09-15T15:46:30Z

Overview

When GGML's Vulkan backend reads an attention cache view in place (the first n rows of a buffer that holds 5120), it assumed the heads were packed back to back while they are actually spaced by the full cache length, so every head but the first read the wrong data and the SheetSage2 decoder looped forever. The fix takes the real stride from the tensor instead of recomputing it from the visible row count, which also repairs the Qwen3 AR --no-fa path on Vulkan, and two new test-backend-ops cases lock it in.

Additional information

Found while chasing ServeurpersoCom/yue2.cpp#4, where the SheetSage2 decoder loops forever on Vulkan while it works on every other backend such as CPU and CUDA. This PR brings Vulkan in line with the others.

Requirements

I have read and agree with the contributing guidelines
AI usage disclosure: YES

jeffbolznv · 2026-09-15T23:02:23Z

    ggml_vk_matmul(
        ctx, subctx, pipeline,
-        { d_X, x_buf_offset, x_sz }, { d_Y, y_buf_offset, y_sz },
+        { d_X, x_buf_offset, x_range }, { d_Y, y_buf_offset, y_range },


Do we even need to set real ranges here? Can we just use ggml_vk_subbuffer?

Done, both A and B are bound with ggml_vk_subbuffer now.

Erratum, reverted this one: the quantized A loads in mul_mm_funcs.glsl have no idx_m < p.M guard and rely on the exact descriptor range to zero fill partial tiles, so binding A and B with ggml_vk_subbuffer made the NVFP4 mul_mm read the previous node's leftovers and hang on NVIDIA without coopmat2 (the vulkan-nvidia-cm job); the range is back to ggml_nbytes for a tensor read in place, which still covers the strided view, and the stream-k PR binds whole buffers on the same path so it may hit this too.

jeffbolznv · 2026-09-15T23:02:59Z

+    const uint64_t x_range = qx_needs_dequant ? x_sz : ggml_nbytes(src0);
+    const uint64_t y_range = (qy_needs_dequant || quantize_y) ? y_sz : ggml_nbytes(src1);
+
+    uint32_t stride_batch_x = qx_needs_dequant ? ne00*ne01 : ggml_vk_batch_stride(src0);


Does mul_mat_id need a similar change?

Yes, same pattern in mul_mat_id, and the strided test_mul_mat_id case I added for it also caught the single token path in mul_mat_vec_id, which passed ne00*ne01 as the batch stride of A; fixed in a separate commit.

jeffbolznv · 2026-09-15T23:04:05Z

+// rows of one batch may be a view into a larger buffer, so the stride comes
+// from nb[2], not from ne[1].
+static uint32_t ggml_vk_batch_stride(const ggml_tensor * tensor) {
+    return (uint32_t)(tensor->ne[0] * (tensor->nb[2] / tensor->nb[1]));


I'm not totally sure what this is doing, but dividing one stride by another is very likely wrong.

For a dim01 contiguous tensor nb[1] is exactly one row, so nb[2]/nb[1] is the number of rows per batch in memory, which exceeds ne[1] when the tensor is a view of a larger cache; I made the comment say that.

I think it is still possible for nb[2] to be padded, even when dim01 are contiguous. I think it would be better to write it as something like nb[2] / type_size * block_size.

Right, nb[2] can be padded, the helper now returns nb[2] / type_size * block_size and a test_mul_mat case with a padded batch stride fails before and passes after.

A dim01 contiguous tensor can still be a view whose batches are strided by more than ne[1] rows, the first rows of a KV cache for example. Both the mat-vec and the matrix paths read such a tensor in place but passed ne00*ne01 as the batch stride, so every head past the first read the wrong rows. The same applies to src1. The stride now comes from nb[2] whenever the tensor is used in place; the value is unchanged for a contiguous tensor. test-backend-ops gets an m_v parameter on test_mul_mat, the number of rows of a in memory, and two cases at the shapes of a decoder self attention over a cache.

The matrix path bound src0 and src1 to the shader with a range of elements times type size, which ends before the batches of a strided view. Pipelines with bounded access read zero past that range, so the same view that the mat-vec path already handles gave wrong results on Intel and on NVIDIA without coopmat2. The range now comes from ggml_nbytes when the tensor is read in place.

Bind the in place A and B of the matrix path with ggml_vk_subbuffer, which spans to the end of the buffer, so a strided view is in range without computing its extent. mul_mat_id reads the batch stride of an in place src0 and src1 with the same helper as mul_mat. test_mul_mat_id gets an m_v parameter, the number of rows of as in memory, and a case whose experts are strided by more rows than it uses.

The single token path of mul_mat_id passed ne00*ne01 as the batch stride of A, so a strided expert view read the wrong rows. The stride now comes from ggml_vk_batch_stride like the other three paths, and src1 follows the same rule. test_mul_mat_id gets a single token case over the strided view.

ServeurpersoCom · 2026-09-17T11:49:00Z

Retested the updated PR in yue2.cpp: without it every Vulkan config reproduces the issue, coopmat2 or not, and with it the transcriptions finish like on CPU and CUDA while the --no-fa LM samples the same tokens as CPU under a fixed seed.

The batch stride of an in place tensor is taken from nb[2] as nb[2] / type_size * block_size, which holds when nb[2] is padded and not a multiple of nb[1]. A test_mul_mat case with a padded batch stride covers it.

Assisted-by: Claude Opus 5

ServeurpersoCom · 2026-09-22T09:16:34Z

Need to fix a regression observed in the CI

The quantized A loads of mul_mm carry no row bound and rely on the descriptor range to read zeros past the last row of a partial tile. Binding A and B up to the end of the buffer let those tiles read the leftovers of a previous node and hung the NVFP4 mul_mm on NVIDIA without coopmat2. The range is the strided extent of a tensor read in place and the staged size otherwise.

@ServeurpersoCom

Cherry-pick of ggml-org/llama.cpp PR ggml-org#28956 (ggml-org#28956) Author: Pascal (@ServeurpersoCom) Reason for our setup: Correctness: prevents silently wrong matmul results when reading tensor slices

…exact in mul_mm)

ServeurpersoCom · 2026-09-27T09:36:41Z

@0cc4m could you take a look when you have a moment? Jeff approved on Sept 18, and the last commit (keep the A and B ranges exact in mul_mm) came after his approval: it fixes the NVFP4 mul_mm hang on NVIDIA without coopmat2 that the CI caught.

…che (ggml-org#28956) * vulkan: read the batch stride of an in place src0 from nb[2] A dim01 contiguous tensor can still be a view whose batches are strided by more than ne[1] rows, the first rows of a KV cache for example. Both the mat-vec and the matrix paths read such a tensor in place but passed ne00*ne01 as the batch stride, so every head past the first read the wrong rows. The same applies to src1. The stride now comes from nb[2] whenever the tensor is used in place; the value is unchanged for a contiguous tensor. test-backend-ops gets an m_v parameter on test_mul_mat, the number of rows of a in memory, and two cases at the shapes of a decoder self attention over a cache. * vulkan: size the in place A and B ranges by their strided extent The matrix path bound src0 and src1 to the shader with a range of elements times type size, which ends before the batches of a strided view. Pipelines with bounded access read zero past that range, so the same view that the mat-vec path already handles gave wrong results on Intel and on NVIDIA without coopmat2. The range now comes from ggml_nbytes when the tensor is read in place. * vulkan: address review from jeffbolznv Bind the in place A and B of the matrix path with ggml_vk_subbuffer, which spans to the end of the buffer, so a strided view is in range without computing its extent. mul_mat_id reads the batch stride of an in place src0 and src1 with the same helper as mul_mat. test_mul_mat_id gets an m_v parameter, the number of rows of as in memory, and a case whose experts are strided by more rows than it uses. * vulkan: read the batch stride of an in place src0 in mul_mat_vec_id The single token path of mul_mat_id passed ne00*ne01 as the batch stride of A, so a strided expert view read the wrong rows. The stride now comes from ggml_vk_batch_stride like the other three paths, and src1 follows the same rule. test_mul_mat_id gets a single token case over the strided view. * vulkan: address review from jeffbolznv The batch stride of an in place tensor is taken from nb[2] as nb[2] / type_size * block_size, which holds when nb[2] is padded and not a multiple of nb[1]. A test_mul_mat case with a padded batch stride covers it. * vulkan: keep the A and B ranges exact in mul_mm The quantized A loads of mul_mm carry no row bound and rely on the descriptor range to read zeros past the last row of a partial tile. Binding A and B up to the end of the buffer let those tiles read the leftovers of a previous node and hung the NVFP4 mul_mm on NVIDIA without coopmat2. The range is the strided extent of a tensor read in place and the staged size otherwise.

…che (ggml-org#28956) * vulkan: read the batch stride of an in place src0 from nb[2] A dim01 contiguous tensor can still be a view whose batches are strided by more than ne[1] rows, the first rows of a KV cache for example. Both the mat-vec and the matrix paths read such a tensor in place but passed ne00*ne01 as the batch stride, so every head past the first read the wrong rows. The same applies to src1. The stride now comes from nb[2] whenever the tensor is used in place; the value is unchanged for a contiguous tensor. test-backend-ops gets an m_v parameter on test_mul_mat, the number of rows of a in memory, and two cases at the shapes of a decoder self attention over a cache. * vulkan: size the in place A and B ranges by their strided extent The matrix path bound src0 and src1 to the shader with a range of elements times type size, which ends before the batches of a strided view. Pipelines with bounded access read zero past that range, so the same view that the mat-vec path already handles gave wrong results on Intel and on NVIDIA without coopmat2. The range now comes from ggml_nbytes when the tensor is read in place. * vulkan: address review from jeffbolznv Bind the in place A and B of the matrix path with ggml_vk_subbuffer, which spans to the end of the buffer, so a strided view is in range without computing its extent. mul_mat_id reads the batch stride of an in place src0 and src1 with the same helper as mul_mat. test_mul_mat_id gets an m_v parameter, the number of rows of as in memory, and a case whose experts are strided by more rows than it uses. * vulkan: read the batch stride of an in place src0 in mul_mat_vec_id The single token path of mul_mat_id passed ne00*ne01 as the batch stride of A, so a strided expert view read the wrong rows. The stride now comes from ggml_vk_batch_stride like the other three paths, and src1 follows the same rule. test_mul_mat_id gets a single token case over the strided view. * vulkan: address review from jeffbolznv The batch stride of an in place tensor is taken from nb[2] as nb[2] / type_size * block_size, which holds when nb[2] is padded and not a multiple of nb[1]. A test_mul_mat case with a padded batch stride covers it. * vulkan: keep the A and B ranges exact in mul_mm The quantized A loads of mul_mm carry no row bound and rely on the descriptor range to read zeros past the last row of a partial tile. Binding A and B up to the end of the buffer let those tiles read the leftovers of a previous node and hung the NVFP4 mul_mm on NVIDIA without coopmat2. The range is the strided extent of a tensor read in place and the staged size otherwise. (cherry picked from commit 5262471)

The vendored fork rebases onto ggml-org/ggml 0.26.0, which now carries the fixes upstreamed from these projects: - Vulkan: mul_mat and mul_mat_id read strided views such as a KV cache slice with their real batch stride, and mul_mm keeps exact A and B ranges (ggml-org/llama.cpp#28956) - Metal: GGML_OP_PAD supports left and circular padding (ggml-org/llama.cpp#29561) - CUDA: bitonic argsort handles rows wider than 1024 without CUB, on HIP and MUSA (ggml-org/llama.cpp#28957) Two commits remain on top of upstream: - cpu: fused snake activation - ggml: conv_1d result layout for batched input (ggml-org/llama.cpp#28738)

… only on 64-byte aligned buffers oneDNN's optimized GPU kernels fail at execute on buffers that are not 64-byte aligned (src/gpu/intel/compute/kernel.hpp check_alignment: "found misaligned buffer"). MUL_MAT_ID's per-expert loop passes dst slices at expert_row_offset * nb1, misaligned when ne0 * 4 is not a multiple of 64: test-backend-ops aborts at MUL_MAT_ID(type_a=f16, m=8, n=16, k=64, m_v=64), the case added with ggml-org#28956, and every MUL_MAT_ID case after it never runs. Fall back to the existing MKL gemm for misaligned pointers (f16, f32 and bf16 row_gemm call sites). Assisted-by: Claude Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ServeurpersoCom requested review from a team and ggerganov as code owners September 15, 2026 15:46

github-actions Bot added testing Everything test related Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Sep 15, 2026

ServeurpersoCom mentioned this pull request Sep 15, 2026

Vulkan problems for SheetSage2 (BPM issues and repeating scores) ServeurpersoCom/yue2.cpp#4

Closed

jeffbolznv reviewed Sep 15, 2026

View reviewed changes

ServeurpersoCom added 2 commits September 17, 2026 11:13

ServeurpersoCom force-pushed the vulkan-batch-stride branch from 08b7e81 to 6ffa3c2 Compare September 17, 2026 09:13

ServeurpersoCom added 2 commits September 17, 2026 12:02

vulkan: address review from jeffbolznv

55ef123

The batch stride of an in place tensor is taken from nb[2] as nb[2] / type_size * block_size, which holds when nb[2] is padded and not a multiple of nb[1]. A test_mul_mat case with a padded batch stride covers it.

jeffbolznv approved these changes Sep 18, 2026

View reviewed changes

SelfRef added a commit to SelfRef/llama.cpp-rdna that referenced this pull request Sep 18, 2026

merge: upstream PR ggml-org#28956

64d01a1

Assisted-by: Claude Opus 5

ServeurpersoCom mentioned this pull request Sep 24, 2026

Add AceStep and OmniVoice shakfu/cyllama#16

Open

SelfRef added a commit to SelfRef/llama.cpp-rdna that referenced this pull request Sep 24, 2026

merge: upstream PR ggml-org#28956 (new head: keep the A and B ranges …

380ad75

…exact in mul_mm)

ServeurpersoCom requested a review from jeffbolznv September 27, 2026 08:46

0cc4m approved these changes Sep 28, 2026

View reviewed changes

0cc4m merged commit 5262471 into ggml-org:master Sep 28, 2026
19 of 21 checks passed

betweentwomidnights mentioned this pull request Oct 3, 2026

vulkan : read the batch stride of an in place mul_mat src from nb[2] betweentwomidnights/ggml#8

Merged

ggerganov mentioned this pull request Oct 5, 2026

ggml : bump version to 0.26.0 ggml-org/ggml#1652

Merged

BrewTestBot mentioned this pull request Oct 5, 2026

ggml 0.26.0 Homebrew/homebrew-core#315641

Merged

1 task

Navigation Menu

Repository navigation

vulkan: fix wrong results when a mul_mat reads a slice of a larger cache - #28956

vulkan: fix wrong results when a mul_mat reads a slice of a larger cache#28956
0cc4m merged 6 commits into
ggml-org:masterfrom
ServeurpersoCom:vulkan-batch-stride

ServeurpersoCom commented Sep 15, 2026

Uh oh!

jeffbolznv Sep 15, 2026

Uh oh!

ServeurpersoCom Sep 17, 2026

Uh oh!

ServeurpersoCom Sep 22, 2026

Uh oh!

jeffbolznv Sep 15, 2026

Uh oh!

ServeurpersoCom Sep 17, 2026

Uh oh!

jeffbolznv Sep 15, 2026

Uh oh!

ServeurpersoCom Sep 17, 2026

Uh oh!

jeffbolznv Sep 18, 2026

Uh oh!

ServeurpersoCom Sep 18, 2026

Uh oh!

ServeurpersoCom commented Sep 17, 2026 •

edited

Loading

Uh oh!

ServeurpersoCom commented Sep 22, 2026

Uh oh!

ServeurpersoCom commented Sep 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ServeurpersoCom commented Sep 15, 2026

Overview

Additional information

Requirements

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ServeurpersoCom commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ServeurpersoCom commented Sep 22, 2026

Uh oh!

ServeurpersoCom commented Sep 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ServeurpersoCom commented Sep 17, 2026 •

edited

Loading