Skip to content

perf(cpu): reject the packed-nibble int4 x int16 acc4 kernel; pin the acc4 int4 accuracy envelope - #1607

Merged
justinchuby merged 7 commits into
mainfrom
squad/roy-int4-i16-acc4
Aug 20, 2026
Merged

justinchuby merged 7 commits into
mainfrom
squad/roy-int4-i16-acc4

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

Negative result: the packed-nibble int4 kernel is 1.5x–2.2x slower

#1590 diagnosed the int4 accuracy_level = 4 route as throwing away its own advantage: prepack_int8_weight widens 4-bit weights to int8 before the dot, so the kernel streams twice the bytes the model actually stores. The obvious fix is a kernel that reads the ONNX nibbles directly.

I built that kernel completely — which is how I can tell you it does not work. It loses in every cell.

This PR is the close-out. Production code is unchanged; what lands is the one durable finding the experiment turned up and the write-up.

What was built (commit 1, kept for auditability)

The full requirement set, not a prototype: consumes the 0.5 B/weight ONNX blob unchanged (no value repack needed — the layout is already [n][k_blocks][blob_size] and block-padded); unsigned nibbles with per-block zero points folded out of the integer dot via Σ(q−zp)a = Σqa − zp·Σa, so an absent zero point is bit-identical to an explicit midpoint; per-group f32 scales; block sizes 16/32/64/128 with exact scalar tails; _mm256_madd_epi16 with four orders of magnitude of i32 overflow headroom; prepack cached by weight identity; gated behind reduced_precision_activation_allowed and accuracy_level == 4.

Correctness was established before timing:

  • bit-exact against a scalar reference over every nibble value in every lane position;
  • exhaustive over all 256 byte values × 32 element positions;
  • matched an independent f64 dequant contract.

All three route guards were verified to fail loud under mutation — including the useful discovery that widening accuracy_level == 4 to >= 0 still left acc0 blocked by the second gate, i.e. the defence-in-depth is real and not redundant.

Why it lost — three independent falsifications

evidence what it kills
flat ratio nibble/int8 is ~1.6 (block 32) / ~2.1 (block 128), flat across an 18x footprint range, 1.6 MB → 29.4 MB a byte-bound kernel's ratio must improve with size; it doesn't
best case still loses in the one cell where the int8 arm spills 64 MiB L3 (73.4 MB) and the nibble arm does not (44.1 MB), nibble is still 1.63x slower the thesis' strongest construction
bandwidth the incumbent runs at 74–77 GB/s = 98–102% of this host's 75.8 GB/s DRAM ceiling at block 128; the nibble kernel reaches 16% you cannot spend bytes you were never able to fetch

Mechanism. _mm256_madd_epi16 retires 16 products per instruction where _mm256_maddubs_epi16 retires 32, so an int16-activation kernel needs 2x the multiplies at the same width; and int16 activations are 2 B, so a fixed 32-weight step reads 2x the activation bytes and needs two loads where int8 needs one. Per 32 weights: int8 ≈ 5 vector ops, nibble ≈ 14. The nibble unpack itself (≈4 ops) is the smaller half of the loss. I also gave it its best shot — folding each group's i32 partial into one f32x8 block accumulator, the 8-bit kernel's structure — and it recovered only 1–3 pp while costing bit-exactness, so that was reverted too.

This is scoped to AVX2. On AVX-512BW _mm512_madd_epi16 retires 32 products, matching maddubs at 256-bit; the doc does not claim the idea is dead there.

The finding worth keeping

Chasing a PARITY_FAIL in the harness turned up something unrelated to performance. At geometry K=4096, N=256, block 32, against f64 truth:

route abs err rel err
nibble kernel 7.94e-4 1.04e-5
incumbent int8 2.19e-1 2.86e-3

The kernel was 276x more accurate — the harness checks agreement with ORT, and ORT makes the same int8 approximation our incumbent does, so a more accurate kernel registers as a parity failure. Worth remembering before anyone reads that warning as "the kernel is wrong."

Following that thread: int4 accuracy_level = 4 quantizes activations to int8 (4.75e-3 rel) while 8-bit accuracy_level = 4 quantizes to int16 (4.89e-7 rel for the exact path) — the same attribute selecting two routes 9,703x apart. Because accuracy_level = 4 is a contract to be less accurate, no output-comparison test can fail when that error drifts, so it was never tracked. accuracy4_int4_decode_error_envelope_is_pinned_against_f64 now pins both sides. The lower bound is the important half: if the asymmetry ever closes, the test fails and forces this record to be revisited rather than silently rotting. Verified non-vacuous by mutation (forcing both arms to level 0 fires it).

What lands

+379 lines, 2 files, no production code change.

  • crates/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs (+151, tests only) — the pin plus an int4_f64_reference helper that reads the ONNX int4 operator literally, independent of every production helper.
  • docs/benchmarks/2026-08-20-int4-nibble-i16-negative.md (+228, new) — full matrix including losses, mechanism, bandwidth table, and reproduction commands.
  • docs/performance/CPU_MATMUL_ASSIGNMENT.md §13 — the ledger entry recording the rejection against §10's premise.

The experiment is preserved as commit 1 so it stays browsable after squash; the kernel is deleted rather than left behind a default-off flag, following the precedent already recorded at borrowed_affine_int4_matmul's precision contract, where an aarch64 int8-activation diversion was removed rather than gated. A flag would not be free — it is a second prepack cache and a live dispatch arm to keep correct forever, in exchange for a route that is flat-dominated across every footprint tested.

Bearing on the remaining ORT gap

ORT does llama3_8b_qkv acc4 m = 1 in 0.18 ms against our 0.361 ms, and our int8 arm is already at 98–102% of DRAM. ORT is not streaming faster. It must touch fewer bytes per output row or reuse more from cache. Weight byte width is now excluded as the lever — which is what this experiment bought, and it redirects the next probe to ORT's working set per output row rather than its byte rate.

Validation

Local CI matrix 19/20 on this branch with origin/main (eb8ce595f) merged in: fmt; offline build; cargo test -p onnx-runtime-ep-cpu (1552 passed, including the new pin); clippy -D warnings over the Linux-offline set, --features native-backend, and --target aarch64-unknown-linux-gnu; --features mlas; --no-default-features; --all-features; the MLAS-free artifact gate; check_cross_compile.sh; all 8 repo lint scripts.

The single failure is --target aarch64-pc-windows-msvc, which needs Windows SDK headers this container lacks (onnxruntime_c_api.h:34:10: fatal error: 'stdlib.h' file not found) and fails identically on unmodified main.

Known gaps, stated plainly

The m = 2..8 and 1/2/4-session concurrency sweeps were not run. The m = 1 loss was 1.5–2.2x across 8 cells against a 0.2–5.1% noise floor, and higher m only amortises the weight stream further in the incumbent's favour, so I closed the route rather than spend hours confirming a decided verdict. If anyone revives this on AVX-512, those remain unmeasured.

Roy and others added 5 commits August 20, 2026 17:53
Consumes the ONNX packed nibbles directly at 0.5 B/weight instead of
expanding each 4-bit weight to a full int8 byte, on the premise that a
decode GEMV is dominated by weight bytes streamed.

This commit is kept so the rejected experiment stays auditable; the next
commit removes it and records the measurement that rejected it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t4 accuracy

The packed-nibble kernel in the previous commit is 1.5x-2.2x slower than
the int8 repack it would replace, in every cell measured. Removed.

The premise was that int4 decode is dominated by weight bytes streamed, so
consuming 0.5 B/weight nibbles instead of a 1 B/weight int8 repack should
roughly halve the time. The measurement falsifies it: nibble/int8 is flat
(1.50-1.63 at block 32) across an 18x range of weight footprint, and in the
one cell where the int8 arm spills L3 (73.4 MB > 64 MiB) while the nibble
arm does not, the nibble arm is still 1.63x slower.

The mechanism is instruction throughput, not bytes. At block 128 the int8
arm already runs at 98-102% of this host's 75.8 GB/s DRAM ceiling, while
the nibble arm reaches 16% -- it is not waiting on memory. An
int16-activation kernel needs 2x the multiply instructions of an
int8-activation one (madd_epi16 retires 16 products, maddubs_epi16 32) and
reads 2x the activation bytes. No AVX2 arrangement closes that.

Kept: accuracy4_int4_decode_error_envelope_is_pinned_against_f64, which
measures against an f64 reading of the operator rather than another kernel.
accuracy_level 4 is a contract to be *less* accurate, so no output
comparison fails when its error drifts and the number was untracked: int4
acc4 carries 4.75e-3 relative error vs 4.89e-7 at level 0, a 9,703x ratio,
because it quantizes activations to int8 while the 8-bit route quantizes to
int16 (~1e-5). The bound is two-sided so that closing that asymmetry fails
the test rather than silently invalidating the doc.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… ledger

Section 10 diagnosed the int4 accuracy_level=4 route as wasting its advantage by
repacking 4-bit weights up to int8. Section 13 records that building the kernel
that premise implies falsified it: the nibble kernel loses 1.5x-2.2x in every
cell, the ratio is flat across an 18x footprint range, and the incumbent is
already at 98-102% of DRAM. Weight byte width is excluded as a lever.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.00990% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.95%. Comparing base (a429538) to head (63c3a0b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...es/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs 99.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1607      +/-   ##
==========================================
+ Coverage   80.79%   80.95%   +0.15%     
==========================================
  Files         380      382       +2     
  Lines      175073   178657    +3584     
  Branches   175073   178657    +3584     
==========================================
+ Hits       141450   144629    +3179     
- Misses      28727    29089     +362     
- Partials     4896     4939      +43     
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (ø)
cli-ort-windows 82.19% <ø> (+0.09%) ⬆️
mlas 85.05% <ø> (?)
offline 80.82% <99.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...es/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs 78.69% <99.00%> (+0.11%) ⬆️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Roy and others added 2 commits August 20, 2026 19:34
…cy pin to its measured route

Review findings on #1607:

- Section 13 credited section 10 with the int8-repack premise. Section 10 is
  about the accuracy_level=0 prefill pack and concludes that pack is *not*
  bandwidth-bound; the premise this experiment falsifies comes from
  docs/benchmarks/2026-08-20-int4-acc4-int8-repack.md. Cited correctly, with a
  note distinguishing the two so a later reader is not sent to the wrong claim.

- The deletion-over-flag precedent was overstated. borrowed_affine_int4_matmul's
  diversion was removed for being semantically wrong for its caller; this kernel
  is correct and more accurate and is dropped purely on speed. Both stated, and
  the decision rests on the flat loss and the maintenance cost of a second
  prepack cache instead.

- The pinned band is measured for the int8-activation route. Non-Apple aarch64
  defaults ONNX_GENAI_CPU_ARM64_INT4_DIRECT on and can send accuracy_level=4 to
  kai_sdot_matmul_m1, a different scheme never measured against this band, so
  the test now asserts it did not take that route rather than silently pinning a
  number it never measured. Verified fail-loud by mutation.

- The bandwidth table's MB column mixes weight-only and full-working-set
  figures; recorded, with the note that the looseness runs against the thesis
  being tested and so changes no conclusion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby

Copy link
Copy Markdown
Owner Author

Review response

Opus review returned approve-with-changes with one concrete defect and three informational notes. All four are addressed in f15b89df0.

1. §13 misattributed its own premise (the real finding). §13 opened with "Section 10's diagnosis was that the int4 accuracy_level = 4 route wastes its advantage by repacking...". That is wrong: §10 is about the accuracy_level = 0 prefill pack and explicitly concludes that pack is not bandwidth-bound (10.8 GB/s, 14% of roofline). The premise this experiment falsifies comes from docs/benchmarks/2026-08-20-int4-acc4-int8-repack.md, landed with #1590 — which the companion doc cited correctly, so the two landed documents disagreed with each other. Verified with grep: prepack_int8_weight and accuracy_level = 4 appear nowhere in §10 or §11. §13 now cites the right source and adds a sentence distinguishing the two claims so a future reader is not sent to a section that does not contain the thing being refuted. Good catch — a wrong cross-reference in a ledger is worse than no cross-reference, because it looks authoritative.

2. The deletion precedent was overstated. I had written that deleting the kernel "follows the precedent" at borrowed_affine_int4_matmul's precision contract. The reviewer is right that this is only a partial analogy: that diversion was removed because it was semantically wrong for its only caller, whereas this kernel is correct and in fact more accurate, and is being dropped purely on speed. §13 now says both things explicitly and rests the decision where it actually belongs — the loss is flat across every footprint tested, so no host or model size argues for keeping it, and a flag would be a second prepack cache plus a live dispatch arm to keep correct forever.

3. The pinned band was measured for one route but not scoped to it. On x86 the fixture is deterministic across the whole Scalar/AVX2/VNNI ladder, because supports_int4_direct requires !has_zero_points there and this fixture quantizes asymmetrically. But non-Apple aarch64 defaults ONNX_GENAI_CPU_ARM64_INT4_DIRECT on and can route accuracy_level = 4 to kai_sdot_matmul_m1 — a different quantisation scheme whose error I have never measured against the 1e-4..1e-2 band. The test now asserts, via KAI_SDOT_M1_TEST_CALLS, that the reduced arm did not take that route, and says so in the failure message. That is the honest option: I cannot pin a number I did not measure, and a mysterious band failure on an aarch64 runner is worse than an explicit "this host took a different route, measure it before sharing this pin." Verified fail-loud by mutation:

the pinned band below was measured for the int8-activation route
(prepack_int8_weight -> int8_matmul); this host dispatched accuracy_level 4 to
the aarch64 kai_sdot direct route instead, which is a different quantisation
scheme and needs its own measurement before it can share this pin
(measured 4.74629609846489e-3)

4. The bandwidth table's MB column mixes two definitions — the nibble arm's 44.1 MB is its full working set while the acc0 row's 29.4 MB is weight bytes alone. Now stated in the doc, along with the reason it changes nothing: the looseness inflates the losing kernel's GB/s, so it runs against the thesis being tested rather than for it.

I did not act on the note that the lower bound fires on a genuine accuracy improvement — that is the intended tripwire, and the assert message already tells the engineer what to do.

Re-validation

Latest origin/main (a429538ff, #1608) merged in and the full matrix re-run from scratch afterwards — 19/20, cargo test -p onnx-runtime-ep-cpu 1552 passed. The only failure is --target aarch64-pc-windows-msvc, which needs Windows SDK headers this container lacks and fails identically on unmodified main.

@justinchuby
justinchuby marked this pull request as ready for review August 20, 2026 19:45
@justinchuby
justinchuby merged commit de977ef into main Aug 20, 2026
6 checks passed
@justinchuby
justinchuby deleted the squad/roy-int4-i16-acc4 branch August 20, 2026 19:45
justinchuby added a commit that referenced this pull request Aug 20, 2026
…ute is never taken (#1611)

Fixes `Rust (Windows ARM64) / Test cross-platform offline crates`, which
is red on `main`.

## What the failure was

```
kernels::matmul_nbits::tests::accuracy4_int4_decode_error_envelope_is_pinned_against_f64
matmul_nbits.rs:10365: the pinned band below was measured for the int8-activation route
(prepack_int8_weight -> int8_matmul); this host dispatched accuracy_level 4 to the aarch64
kai_sdot direct route instead ... (measured 2.839095278992024e-3)
```

Introduced by #1607 (`de977ef0`), which landed this test. It is
**deterministic on that job, not flaky.**

The test asserted that `accuracy_level = 4` did *not* dispatch to
`kai_sdot_matmul_m1`, reasoning that the pinned `1e-4..1e-2` band was
measured for the int8-activation route and the kai_sdot route's error
was unknown. That reasoning is correct and worth preserving. The problem
is the platform assumption underneath it:
`arm64_kai_sdot_direct_enabled()` defaults to **on** for non-Apple
aarch64, and `Rust (Windows ARM64)` runs `onnx-runtime-ep-cpu`'s tests
on exactly that default. The assertion could never hold there.

## How I reproduced it

The test is not `cfg`-gated to x86_64, and the route is env-controlled,
so Apple aarch64 can be forced onto the same path CI takes:

```
ONNX_GENAI_CPU_ARM64_INT4_DIRECT=1 cargo +1.98.0 test -p onnx-runtime-ep-cpu --lib \
  accuracy4_int4_decode_error_envelope
```

That reproduces the CI failure exactly — same panic, and the same
measured value `2.839095278992024e-3` as the Windows ARM64 job log, to
every digit.

## What changed

That reproduction supplies the measurement the test said was missing,
which turns out to make the skip unnecessary:

**The kai_sdot route measures `2.839095278992024e-3`, which is inside
the existing band** — and it is bit-identical on Windows ARM64 and on
Apple aarch64 with the route forced. So the band is now measured on two
independent aarch64 hosts and one x86 ladder, and it holds on all of
them.

So rather than exempt the kai_sdot host from the band, the band is
asserted on **both** routes, and the route name is folded into the
failure message so a future regression reports which quantisation scheme
it measured. The `!reduced_took_kai_sdot` assertion is removed; nothing
else about the test weakens, and no assertion becomes conditional.

The doc comment is updated to record the measurement and the two hosts
it was taken on.

## How I verified it

Negative control, both directions:

| Config | Before | After |
|---|---|---|
| `ONNX_GENAI_CPU_ARM64_INT4_DIRECT=1` (the CI ARM64 route) | **FAILED**
| **ok** |
| default (int8-activation route) | ok | ok |

Full crate suite, both configurations:

- `cargo +1.98.0 test -p onnx-runtime-ep-cpu --lib` → **1490 passed, 0
failed**
- `ONNX_GENAI_CPU_ARM64_INT4_DIRECT=1 cargo +1.98.0 test -p
onnx-runtime-ep-cpu --lib` → **1490 passed, 0 failed**

`cargo +1.98.0 fmt --all -- --check` clean. Toolchain 1.98.0 chosen to
match CI (`rustc 1.98.0 (88d9e12ae 2026-08-18)`, read from the job
logs).

## What I could not verify

- **I have not run this on Windows ARM64.** The reproduction is Apple
aarch64 with the route forced on. The identical measured value across
the two hosts is strong evidence they execute the same kernel, but it is
inference, not a run on the target. CI is the oracle.
- The band is now pinned for kai_sdot on the strength of **one fixture**
(`k=1024, n=128, block_size=32`, asymmetric). That is the same
evidentiary basis the int8 route's band already had, but it is not a
sweep, and I am not claiming it is.
- I did not investigate the `plugin_ort_e2e-….exe` `0xc0000005
STATUS_ACCESS_VIOLATION` previously seen on this same job. That is a
separate defect and is not addressed here.

## Notes

- Touches only `crates/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs`,
in one test and its doc comment. No production code paths change.
- Independent of #1610, which fixes a structurally identical defect (a
test pinning a dispatch route the host does not take) in `matmul.rs`.
Different file, no overlap.

Refs #1600.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant