[MOD-17527] Widen and unsign the plain uint8 integer accumulators

[dor-forer]

Describe the changes in the pull request

The uint8 kernels accumulate products or squared differences of bytes, so the total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold that, and all of them are on the plain int8/uint8 index paths that ship today — unlike the SQ8 work in #1011, which is latent because nothing on main constructs an SQ8 index.

The accumulation itself was never wrong. The SIMD adds wrap modulo 2³² and are bit-exact, so the bit pattern was already correct; the top bit was being read as a sign. Reading it as unsigned costs nothing and doubles the exact range from dim 33,025 to 66,051. Past that the reduce is widened to 64 bits where that is free or cheap, and above the point where the lanes run out the scalar kernel takes over.

Stack position 1 of 3, base main:

main
 └── this PR    uint8 accumulators        <- you are here
      └── #1015   quantize numerical safety
           └── #1011  exact SQ8 metadata + kernels

Which issues this PR fixes

1. MOD-17527

Main objects this PR modified

1. ret_t in IP.cpp / L2.cpp — 64-bit for every element type.
2. UINT8_InnerProductImp in the four IP uint8 kernels — returns the exact integer dot.
3. UINT8_L2SqrSIMD* in the four L2 uint8 kernels — unsigned/widened reduce.
4. spaces.h — MAX_EXACT_UINT8_SIMD_DIM, the single dimension bound.
5. The three uint8 choosers in IP_space.cpp / L2_space.cpp — scalar fallback above it.
6. The four IP_*_SQ8_SQ8.h kernels — they reuse the shared helper, so they take its result as uint32_t.
7. tests/unit/test_spaces.cpp, tests/benchmark/spaces_benchmarks/bm_spaces_uint8.cpp.

Mark if applicable

• This PR introduces API changes
• This PR introduces serialization changes

---

The four defects

#	where	before	broke from dim
1	scalar IP.cpp / L2.cpp	ret_t = int for 1-byte elements	33,026 — signed-overflow UB
2	UINT8_InnerProductImp NEON / SVE	accumulated exactly, then return static_cast<float>	258 (2²⁴ / 65025)
3	UINT8_InnerProductImp AVX-512	_mm512_reduce_add_epi32 into a signed int	33,026
4	UINT8_L2Sqr AVX-512 / NEON	unsigned reduce read back as signed int	33,026 — distance went negative

On (1), the conditional was also dead: std::conditional_t<sizeof(int_elem_t) == 1, int, long long> only ever sees int8_t and uint8_t, both 1 byte, so it always selected int and the long long branch was unreachable. The comment above it claimed "support to 2^16", but int reaches only 33,025. ret_t is now unconditionally 64-bit, which also covers int8 at dim 131,072. Kept signed so the 1 - ip in the wrappers stays signed arithmetic and cannot underflow.

One bound, on the horizontal total

MAX_EXACT_UINT8_SIMD_DIM = 66,051 = floor(UINT32_MAX / 65,025)

Above it the choosers hand back the scalar kernel, which is exact after the ret_t change. One comparison at index creation, reusing the if (dim < 32) return ret_dist_func idiom already there, and every kernel is left untouched.

The bound is on the horizontal total, which does not depend on how products land in individual lanes. That is the property that makes it verifiable in one line, and it is why a wider bound was abandoned — see below.

Three alternatives tried and rejected, each on evidence

Widening the reduce unconditionally. Measured on an Ice Lake-SP Xeon, 15 repetitions, pinned core, two passes with the A/B order reversed: +20% at dim 32, +8–11% across 55–200, +4–5% at 900–1024, on byte-identical loop code. Four extra uops in a dependency chain, which is why instruction count understates it.

A runtime branch selecting the width per call. Measured at a constant +0.4 to 0.6 ns per call, +15% at dim 32. The mechanism was not the branch alone: the fatter function lost its inlining in 31 of the 65 Cosine wrappers and .text grew 18.4%.

Compile-time selection between two named wrappers, extending SIMD to a second bound of 4 × 66,051. This one was genuinely free on the narrow path — verified, the narrow wrappers stayed byte-identical and the out-of-line count unchanged. It was rejected for correctness, not cost, and the margin is not close:

per-lane total at dim 264,204, spread evenly	4,294,966,275
UINT32_MAX	4,294,967,295
headroom for any unevenness	1,020
worst-case extra in one lane from the masked residual load (16 products)	1,040,400

The bound assumed products spread evenly across the four uint32 lanes after NEON's 32-bit vaddq_u32 merge. That holds for the main loop and fails once the residual path is counted, so lanes wrap before vaddlvq_u32 sees them. A corrected bound is derivable — about 264,140 here — but only per kernel, from its accumulator count and residual distribution, and any later change to a residual path breaks it silently. Found by Cursor Bugbot.

Chunked accumulation, flushing into a 64-bit total, would be exact at any dimension and needs no lane reasoning. Deferred: it restructures the loop in eight kernels, four of them ARM, and the reach has no known use.

Recorded for whoever revisits this: on ARM the widening reduce is free instruction-for-instruction. clang++ --target=aarch64-linux-gnu -O2 emits

narrow:  addv   s0, v0.4s  ->  fmov w8, s0  ->  ucvtf s0, w8      3 instructions
wide:    uaddlv d0, v0.4s  ->  fmov x8, d0  ->  ucvtf s0, x8      3 instructions

So the obstacle to a wider band is the lane bound, not the reduce.

Verification

Against the narrow-only version, the narrow wrappers are byte-identical:

	narrow-only	this PR
IP res 0 / 32 / 33	33 / 40 / 47	33 / 40 / 47
Cosine res 0 / 32 / 33	37 / 43 / 50	37 / 43 / 50
L2 res 0 / 32	33 / 41	33 / 41
call in the 33–63 band	0	0
out-of-line Imp copies	7	7

Residual 33–63 is checked specifically because that is the band the runtime branch broke. Object file grows 514,792 → 656,120 bytes for the 192 extra instantiations, which is the intended trade.

Also: -fsyntax-only -Wall -Wextra clean on both scalar TUs and the test file; check-format.sh clean.

The "free on ARM" claim is verified, not assumed. Cross-compiling the reduce construct with
clang++ --target=aarch64-linux-gnu -O2 emits the same instruction count either way:

narrow:  addv   s0, v0.4s  ->  fmov w8, s0  ->  ucvtf s0, w8      3 instructions
wide:    uaddlv d0, v0.4s  ->  fmov x8, d0  ->  ucvtf s0, x8      3 instructions

Same kinds, same functional unit, differing only in register width, which costs nothing on AArch64.
That is why the ARM kernels carry no narrow variant: there would be nothing to choose between. A
two-name treatment was implemented there and then reverted once this measurement existed.

Not built or executed locally; the full ARM build is covered by CI only.

Tests

UINT8_is_exact_across_every_dimension_band walks all four boundaries — each bound and one dimension past it — through the dispatched function, so selection is covered as well as arithmetic. All-255 against all-0 is the worst case and keeps every expectation an exact integer.

UINT8_dispatchers_fall_back_to_scalar_past_the_simd_dim asserts the top boundary by pointer identity, because on a host without a uint8 SIMD tier the value comparisons would pass either way. It also asserts that the narrow and widened dispatch results are different functions, so the selection is exercised rather than assumed.

The existing UINT8 suites run testing::Range(32, 129), which is why all of this went unseen — and being SIMD-versus-scalar comparisons they would have agreed with each other wherever both wrapped.

Also fixed

The uint8 spaces benchmark fixture paired new[] with delete and stored the trailing norms through unaligned float casts. Any measurement previously taken from it was untrustworthy, which matters because the numbers above come from it.

Known, not fixed here

The SQ8_SQ8 choosers have no dimension guard, unlike the uint8 ones, so those kernels are selectable at any dimension. SQ8 is independently capped at 66,051 by its uint32 q_sum_squares metadata slot, so the fence belongs with SQ8 index creation in #1007. Unreachable on main.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes core distance kernels used by vector indexes; wrong math would skew ANN results, but behavior is narrowly scoped to uint8 paths and is heavily regression-tested at high dimensions and chunk boundaries.

Overview
Fixes incorrect uint8 inner-product and L2 distances when dimension pushes dot-product or squared-difference totals past what 32-bit signed accumulators could represent (notably from ~33k dims, with negative L2 in some SIMD paths).

Scalar: ret_t in IP.cpp / L2.cpp is now 64-bit with unsigned types for uint8, and UINT8_InnerProduct converts the integer total to float before 1 - ip.

SIMD (AVX-512 VNNI, NEON, NEON_DOTPROD, SVE): Kernels return an exact uint32_t chunk total (unsigned reduce) instead of signed/float intermediates; L2 paths split into *Imp + float wrapper. For dim > spaces::UINT8_CHUNK_ELEMENTS (65536), new chunked IP/L2/Cosine variants sum exact 32-bit chunk totals in 64 bits; implementation choosers pick chunked vs plain once per index (no per-call branch). Helpers use always_inline / static where needed so chunked call sites do not break inlining.

SQ8↔SQ8 dot paths take uint32_t from the shared uint8 helper (still single-chunk 32-bit there; dimension cap noted for SQ8 elsewhere).

Adds UINT8_CHUNK_ELEMENTS in spaces.h, unit tests through chunk boundaries and all residuals/tiers, and small fixes in the uint8 spaces benchmark (delete[], aligned norm storage).

^{Reviewed by Cursor Bugbot for commit a6db25f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.16%. Comparing base (efd63da) to head (a6db25f).
⚠️ Report is 1 commits behind head on main.

⚠️ Current head a6db25f differs from pull request most recent head 8a9bfc6

Please upload reports for the commit 8a9bfc6 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1014      +/-   ##
==========================================
- Coverage   97.17%   97.16%   -0.01%     
==========================================
  Files         141      141              
  Lines        8328     8374      +46     
==========================================
+ Hits         8093     8137      +44     
- Misses        235      237       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 3be2884. Configure here.}