Skip to content

Latent int16 overflow in ggml_vec_dot_i2_i8_s_1x1: 128 maddubs results per lane against a 32767 ceiling #629

Description

@purpleskulll

Summary

ggml_vec_dot_i2_i8_s_1x1 accumulates 128 vpmaddubsw results into one
int16 lane before folding. The worst case is 128 * 508 = 65024 against a
32767 ceiling. It is correct in practice only because signed activations
cancel, and it is latent on a default build because the sgemm path takes
the work instead.

I am filing it as hardening, not as a wrong-output bug, and the distinction is
deliberate — see Severity below.

The arithmetic

ggml/src/ggml-cpu/quants.c:

const int group32_num = nb / 32;                     // :1396
...
__m256i accu32 = _mm256_setzero_si256();             // :1413  (int16 lanes)
for (int j = 0; j < 32; j++) {                       // :1414
    ...
    accu32 = _mm256_add_epi16(accu32, _mm256_add_epi16(xq8_0, xq8_1));   // :1439
    accu32 = _mm256_add_epi16(accu32, _mm256_add_epi16(xq8_2, xq8_3));   // :1440
}
accu = _mm256_add_epi32(_mm256_madd_epi16(accu32, one16), accu);         // :1445

32 iterations × 4 planes = 128 vpmaddubsw results per int16 lane. Each is at
most 2 * 2 * 127 = 508, so the lane can reach 65024, roughly twice the
ceiling. The name accu32 suggests 32-bit; the type is __m256i used with
_mm256_add_epi16.

What actually happens

With all activations the same sign the sum does not cancel and the lane wraps.
Measured on the real weights of BitNet-b1.58-2B-4T — 30 tensors at
K = 6912, driven against int8 activations pinned to +12711,998 of
12,000 rows
disagree with exact int64 arithmetic, and every error is a
multiple of 65536, the signature of a wrapped int16.

With ordinary int8 activations, 0 of 12,000 rows wrap. That is why it has
never been seen.

Severity: low, and I want to be accurate about why

  • On a build where llamafile_sgemm handles these shapes this function is not
    called at all. The kernel's own counters on my build read
    calls=0 sgemm=52068.
  • It becomes reachable when that path is disabled, which is a supported switch
    rather than an exotic one.
  • Even then it needs same-signed activations across a whole group of 32 blocks.

So: a latent overflow in a fallback, not a correctness bug users are hitting.
I would not have filed it on its own; I am filing it because it is the same
missing bound-derivation as the fold issue I opened alongside this one (#628), in the
same file family, and a fix for one is naturally a fix for both.

Not a duplicate of the ARM reports

#411 and the audit in #525 report an int16 overflow, but it is the NEON
vmlal_s8 accumulation in src/ggml-bitnet-mad.cpp, which does produce visible
garbage. This is a different function, a different instruction set and a
different failure mode — this one is invisible precisely because it cancels.

Suggested fix

Fold every 8 groups rather than every 32, or accumulate in int32 directly.
Upstream ggml's equivalent kernels derive the bound explicitly and stay inside
it — TQ2_0 in arch/x86/quants.c carries // 16-bit sums, because 256*127 still fits — which is the pattern worth copying here.

Reproducible from https://github.com/purpleskulll/bitnet-t5b.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions