Skip to content

ggml-cpu: Incorrect A indexing in simd_gemm scalar tail-column path - #25390

Merged
ggerganov merged 1 commit into
ggml-org:masterfrom
tyronecai:patch-1
Jul 8, 2026
Merged

ggerganov merged 1 commit into
ggml-org:masterfrom
tyronecai:patch-1

Conversation

@tyronecai

@tyronecai tyronecai commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Overview

simd_gemm() has an incorrect A-matrix index in the scalar tail-column path for full row blocks.

Additional information

This appears to have been introduced by commit 08e6d914b8fef477d2a5aeeb89e08d42709981cf (ggml : avoid UB in gemm ukernel (llama/19642)), which changed the SIMD GEMM ukernel from using global ii / jj offsets to passing already-offset A, B, and C pointers.

image

Before that commit, the scalar tail-column path used the global row index:

a += A[(ii + i) * K + kk] * B[kk * N + jj];

After the commit, A is advanced after each full row block:

A += GEMM_RM * K;
C += GEMM_RM * N;

So inside the current block, the correct row-major index should be:

A[i * K + kk]

Requirements

`simd_gemm()` has an incorrect A-matrix index in the scalar tail-column path for full row blocks.
@tyronecai
tyronecai requested a review from ggerganov as a code owner July 7, 2026 07:47
@github-actions github-actions Bot added the ggml changes relating to the ggml tensor library for machine learning label Jul 7, 2026
@ggml-gh-bot

ggml-gh-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

Hi @tyronecai, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • PR Template not respected: Please respect the template when creating a new pull request. Make sure to fill out all required sections.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that.

@ggerganov

Copy link
Copy Markdown
Member

For reference, this was the PR #19642

@ggerganov ggerganov added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Jul 7, 2026
@ggerganov
ggerganov merged commit 57b50e1 into ggml-org:master Jul 8, 2026
25 of 27 checks passed
@tyronecai
tyronecai deleted the patch-1 branch July 8, 2026 07:06
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 12, 2026
…25390)

`simd_gemm()` has an incorrect A-matrix index in the scalar tail-column path for full row blocks.
zbrad pushed a commit to zbrad/llama.cpp that referenced this pull request Sep 10, 2026
…25390)

`simd_gemm()` has an incorrect A-matrix index in the scalar tail-column path for full row blocks.
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 merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants