Repository navigation
test(cpu): stop the bf16 prefill guardrail asserting a route AVX-512 hosts cannot take - #1610
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1610 +/- ##
==========================================
- Coverage 80.94% 80.93% -0.01%
==========================================
Files 382 382
Lines 178395 178668 +273
Branches 178395 178668 +273
==========================================
+ Hits 144401 144607 +206
- Misses 29061 29117 +56
- Partials 4933 4944 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…hosts cannot take
`Test cross-platform offline crates` fails on
`half_prefill_gebp_agrees_with_the_blocked_half_gemm_and_is_the_route`:
assertion `left == right` failed: BFloat16 m=2: prefill did not take the
fused widen-pack GEBP
left: 0
right: 1
The library is right and the test is wrong. `half_gemm_tile` tries the native
AVX-512 BF16 kernel *before* the fused widen-pack GEBP:
if format == HalfFormat::Bf16 && x86_bf16::native_available() { ...; return }
if half_prefill_gebp_selected(..) && half_prefill_gebp_enabled() { ... }
`native_available()` is a runtime `is_x86_feature_detected!("avx512bf16")`
probe, so on a host that has AVX-512 BF16 a bf16 tile legitimately never
reaches the GEBP and the counter stays 0. The test carved out the analogous
MLAS/f16 case but not this one.
That explains the shape of the failure, which otherwise looks like a
regression from whatever merged most recently: it tracks the runner's CPU, not
the commit. It passed on `eb8ce595` and failed on `a429538f` on Linux while
also failing on Windows x86_64 — different hardware, same tree. Nothing in
#1608 (CUDA VMM) could have touched a CPU bf16 GEBP route.
The precedence is already documented and already encoded for the decode side
in `half_decode_prefers_gebp_when`, which declines bf16 exactly when
`native_available()`. Only the prefill guardrail was missing it.
Rather than switch the assertion off on AVX-512 hosts, count both arms and
assert exactly one ran. A bare "GEBP was not taken" skip would stop catching a
silent fall-through to the portable blocked half GEMM on precisely the newest
hardware; the disjunction keeps the dispatch guarded on every CPU. The numeric
agreement check against `naive_matmul` runs either way, as before.
Verification, and its limits:
- `cargo fmt --all -- --check` clean, and `cargo clippy -p onnx-runtime-ep-cpu
--all-targets` reports only the pre-existing macOS lints fixed in #1609 —
none from this change.
- **The changed code is not compiled locally at all.** It is entirely
`#[cfg(target_arch = "x86_64")]` and this host is aarch64-apple-darwin. I
tried both x86_64 targets: `x86_64-apple-darwin` fails because upstream
publishes no `onnxruntime-osx-x86_64-1.28.0` asset, and
`x86_64-unknown-linux-gnu` fails in ort-sys bindgen with `'stdlib.h' file not
found` — the same environmental blocker #1604 hit on
`aarch64-pc-windows-msvc`. `onnx-genai-ort-sys` is a normal dependency of
`onnx-runtime-ep-cpu` via `onnx-runtime-ep-api`, so there is no `--lib`-only
escape. CI is the oracle for this one.
- Even a green CI run only proves the non-AVX-512 arm unless the runner
happens to have `avx512bf16`. The `(0, 1)` branch may go unexercised.
Refs #1600.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…dicates The guardrail asserted `half_prefill_gebp_calls() == 1` unconditionally, but which route `half_gemm_tile` takes is a property of the host, not the kernel. Two predicates it dispatches on bite on real runners: * `x86_bf16::native_available()` -- the native AVX-512 BF16 arm is tried *before* the GEBP and returns, so a bf16 tile on an `avx512bf16` host never reaches the GEBP. GitHub's Linux x64 pool is a mix of Intel generations, so the same commit passed or failed depending on which runner picked it up. A naive bisect fingers the commit that happened to land on a newer runner. * `has_simd_x86()` -- without AVX2+FMA neither fast route is selected. The test already handled this with a `continue`, which also skipped the numeric comparison against the naive matmul. That is the half of this test worth running on every host, so it now stays on the unconditional path and the no-SIMD case is asserted as a legitimate `(0, 0)`. Rather than restate those hardware assumptions, the expectation is now computed from `half_prefill_gebp_selected` / `half_prefill_gebp_enabled` / `native_available` themselves, so it cannot drift from the dispatch order. A test-only counter on the native bf16 arm distinguishes "took the other fast route" from "silently fell through to the portable blocked half GEMM", which is what the guardrail is actually for. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
justinchuby
force-pushed
the
justinchuby-fix-bf16-gebp-guardrail
branch
from
August 20, 2026 20:39
5a58aab to
e867f8f
Compare
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>
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
Rust coveragefailure inkernels::matmul::tests::half_prefill_gebp_agrees_with_the_blocked_half_gemm_and_is_the_route.What the failure was
This is a latent test defect, not a regression, and it is not attributable to any commit. A naive bisect says otherwise —
Rust coverage (Linux x86_64)is green ateb8ce595(#1604) and red ata429538f(#1608) — but #1608 changes exactly one file,crates/onnx-runtime-ep-cuda/src/provider.rs, and that coverage job's package list does not contain-p onnx-runtime-ep-cudaat all. The commit's only changed file is not compiled by the job that went red. The red tracks which runner picked up the job, not the code.half_gemm_tiletries the native AVX-512 BF16 kernel before the GEBP and returns:On a host with
avx512bf16(Cooper Lake / Sapphire Rapids and later) a bf16 tile legitimately never reaches the GEBP and the counter stays 0. GitHub's Linux x64 pool is a mix of Intel generations, so the same commit passes or fails depending on the runner. The precedence is already encoded for decode inhalf_decode_prefers_gebp_when; only prefill's guardrail lacked it.A second, latent hardware assumption in the same test
The test already had
if !crate::backend::has_simd_x86() { continue; }. That is acontinue, so on a non-AVX2/FMA host it skipped the numeric comparison againstnaive_matmulas well as the route assertion — deleting the half of the test that is valuable on every host. It has never fired (all current runners have AVX2), so it was latent, but it is the same class of bug.What changed
The expectation is now derived from the predicates
half_gemm_tileactually dispatches on, rather than restating hardware conditions in the test:HALF_NATIVE_BF16_CALLScounter on the native bf16 arm distinguishes "took the other fast route" from "silently fell through to the portable blocked half GEMM" — which is what the guardrail is for. Without it, the only way to tolerate the bf16 case is to stop asserting anything.(0, 0)on a no-SIMD host is now an asserted-correct outcome instead of a skip.naive_matmulnow runs unconditionally on every host.How I verified it
Transcribed
half_gemm_tile's dispatch order and the new expectation into a standalone program and checked them against each other across all 16 combinations ofnative_available×has_simd_x86×half_prefill_gebp_enabled× dtype. They agree on all 16.What I could not verify
I have not reproduced the original failure, and I have not run this test. Stating that plainly rather than implying otherwise.
This host is aarch64 macOS; the test is
#[cfg(target_arch = "x86_64")]andx86_bf16::native_available()does not exist here. Cross-compiling is blocked in both directions:x86_64-apple-darwinbecause upstream publishes noonnxruntime-osx-x86_64-1.28.0.tgz(HTTP 404), andx86_64-unknown-linux-gnubecause ort-sys bindgen fails on'stdlib.h' file not found.onnx-genai-ort-sysis a normal dependency viaonnx-runtime-ep-api, so--libdoes not avoid it. No Docker on this machine.So the 16-configuration check verifies the logic is consistent with the dispatch order. It does not verify that the code compiles under
target_arch = "x86_64", nor that the counter increments where I placed it. CI is the oracle for both.A green run on this PR will not prove the fix. It only exercises the
(1, 0)branch unless the runner happens to haveavx512bf16. The(0, 1)branch — the one that was actually failing — is reachable only on Cooper Lake or newer, and a runner cannot be requested. Corroborating evidence for the mechanism, though: #1609's run went green onRust coverage (Linux x86_64)andRust coverage (Windows x86_64)with the old assertion still in place, which is exactly what the CPU-dependence diagnosis predicts and is not evidence that the old test was fine.Notes
crates/onnx-runtime-ep-cpu/src/kernels/matmul.rs. No overlap with Merge the #1186 memory stack (phases 1-7) onto main #1579's file set; nothing incrates/onnx-genai-engine/src/{decode,native_decode}/.a429538f, which predates fix(ci): clear the remaining Rust 1.98.0 clippy failures (qmoe, ep-cpu) #1609, so this branch inheritsmain'sCUDA compile (Linux x86_64)red atClippy CUDA execution provider. That failure is not from this change — fix(ci): clear the remaining Rust 1.98.0 clippy failures (qmoe, ep-cpu) #1609 fixes it.Refs #1600.