Skip to content

cuda: guard cublas GEMM against view-tensor stride false positives - #374

Open
powellste wants to merge 1 commit into
TheTom:feature/turboquant-kv-cachefrom
powellste:fix/cublas-view-tensor-stride
Open

powellste wants to merge 1 commit into
TheTom:feature/turboquant-kv-cachefrom
powellste:fix/cublas-view-tensor-stride

Conversation

@powellste

Copy link
Copy Markdown

ggml_is_contiguously_allocated() returns true for view tensors whose nb[] is inherited from the parent, causing the contiguous-branch stride scaling (s01 *= src0_bs) to produce s01 < ne10. cuBLAS then returns CUBLAS_STATUS_INVALID_VALUE on the GEMM call.

Two fixes:

  1. Guard the contiguous allocation path with !src0->view_src / !src1->view_src so view tensors always take the non-contiguous path, which correctly recomputes s01 = ne00 after conversion.

  2. Add an explicit lda >= k / ldb >= k invariant check before every GEMM call, logging the tensor name, type, view_src, nb[1], and dimensions so any future stride mismatch is immediately diagnosable.

Tested: llama-server starts and responds to inference requests with MTP active (draft_n=2, draft_n_accepted=2).

Root cause: ggml_is_contiguously_allocated() returns true for view tensors
whose nb[] is inherited from the parent, causing the contiguous-branch stride
scaling (s01 *= src0_bs) to produce s01 < ne10. cuBLAS then returns
CUBLAS_STATUS_INVALID_VALUE on the GEMM call.

Fix 1 (lines 1554, 1583): guard the contiguous allocation path with
!src0->view_src / !src1->view_src so view tensors always take the
non-contiguous path which correctly recomputes s01 = ne00 after conversion.

Fix 2 (after line 1643): add an explicit lda >= k / ldb >= k invariant
check before every GEMM call, logging the tensor name, type, view_src,
nb[1], and dimensions so future failures are immediately diagnosable.

Tested: builds clean (no warnings), llama-server starts and responds to
inference requests with MTP active (draft_n=2, draft_n_accepted=2).
@TheTom

TheTom commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Thanks for moving this onto its own branch. I'm closing #366 in favor of this one so the discussion lives in one place.

The request from #366 still applies before this can merge: the exact tensor that reaches CUBLAS_STATUS_INVALID_VALUE (name, type, ne, nb, view offset, and which op created the view), the model and command line, and a test-backend-ops case that fails on base and passes with the patch. Two reasons it matters here:

  • For a view of whole rows, ggml_is_contiguously_allocated is correct and the direct convert path is the right one, so !src0->view_src sends every such view (row slices of a KV cache, slices of a stacked weight) through the slower strided copy. If the tensor that trips this has strides that do not describe its own layout, the bug is in whatever built that view, and the reproducer will show where.
  • The s01 < ne10 / s11 < ne10 checks are useful on their own as a diagnostic, since cuBLAS rejects those leading dimensions anyway and the new log line names the tensor. I'm happy to take that half separately if you want to split it out.

If it only reproduces with a specific GGUF, a link plus the llama-server command line is enough for us to build the test case from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants