Repository navigation
perf(cpu-ep): ship the decode f32 GEMV that #1091 left switched off (6.85x -> 1.15x vs ORT) - #1183
Merged
Merged
Conversation
…6.2-7.7x) #1091 added a native `M = 1` SGEMV to the `SimdX86` backend -- MLAS's `SgemmKernelM1Avx` mechanism, absorbed: stream B in place, no packed panel, because at one output row every byte of B is read exactly once and a packed copy is reused zero times. It shipped behind `ONNX_GENAI_CPU_MM_SIMD_M1_GEMV`, default off, "until the win is measured". Nothing measured it, so every decode `MatMul` in the default build kept paying the packed GEBP path: a full `K * N` read-and-write copy of B for no reuse, driven by a `6x16` microkernel with five of its six rows idle. Measured now, both levels, and made the default. The env probe is deleted -- it was also a locked `std::env::var` lookup on every SGEMM call. Kernel level (`bench_f32_gemm_ab`, 1 rayon thread, `taskset -c 8-15`, min-of-5, Qwen2.5-14B f32 shapes; packed/gemv, higher means the GEMV wins): | shape | packed | GEMV | ratio | |--------------------|----------|----------|-------| | 1x5120x7168 QKV | 31.889m | 5.051m | 6.31x | | 1x5120x5120 o | 20.517m | 3.328m | 6.17x | | 1x5120x13824 gate | 68.102m | 8.911m | 7.64x | | 1x13824x5120 down | 65.828m | 10.169m | 6.47x | | 1x5120x152064 head | 734.203m | 95.901m | 7.66x | | CTL 128x5120x5120 | 108.535m | 111.192m | 0.98x | | CTL 128x5120x13824 | 292.547m | 291.881m | 1.00x | | CTL 128x13824x5120 | 257.195m | 261.052m | 0.99x | The `M = 128` rows are the harness's built-in control: the M=1 route cannot reach them, so they measure the run's noise, and all three are within 2%. Session level, ORT on both sides (`plugin_path_ab_vs_plain_ort`, `1x2048x2048`, 31 interleaved iterations, both pools pinned). ours/ORT, lower is better: | threads | M | p50 before | p50 after | p90 before | p90 after | |---------|---|------------|-----------|------------|-----------| | 1 | 1 | 5.63 | **1.11** | 5.71 | **1.12** | | 1 | 128 | 1.05 | 1.02 | 1.05 | 1.02 | | 4 | 1 | 5.33 | **2.25** | 4.90 | **2.35** | | 4 | 128 | 1.58 | 1.55 | 1.57 | 1.58 | At 8 and 16 threads the M=128 control moved 8.4x and 1.4x between the arms on this shared host, so no conclusion is drawn from those runs. What remains at M=1 is the thread-scaling gap that already has its own section in the doc: 1.11 at one thread, 2.25 at four. Numerics: the two routes sum the same products in a different order, so f32 results at `M = 1` move within the tolerance `m1_route_matches_packed_within_tolerance` already pins -- that test now names both routes explicitly instead of calling `sgemm_simd` for the packed side, which after this change would have compared the GEMV with itself, and it asserts the two routes are not bit-identical so the tolerance cannot pass vacuously. Each route is deterministic on its own: disjoint column strips, a fixed K-unroll order, same bits on every run and every thread count. Two new guards, because nothing else in the file failed when the shipped route was the wrong one: `the_default_entry_point_routes_m1_to_the_gemv` pins the public entry to the GEMV bit-for-bit at M=1 and to the packed kernel bit-for-bit at M=2, and `no_environment_variable_can_change_the_m1_route` pins that the route is no longer readable from process state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Setting `ONNX_GENAI_CPU_MM_SIMD_M1_GEMV` to prove it is unread is a data race: this binary runs its cases in parallel beside dozens of live `env::var` readers, which is the exact hazard `provider.rs` already forbids. The test could not fail for its own regression anyway -- it set `"0"`, which was the old default, so a reintroduced probe would have agreed with it. The route is a compile-time constant and `the_default_entry_point_routes_m1_to_the_gemv` already pins it bit-for-bit with an M>=2 control, without touching process state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
marked this pull request as ready for review
August 18, 2026 04:39
justinchuby
enabled auto-merge (squash)
August 18, 2026 04:39
|
| Status | Scenario | Base | PR | Change |
|---|---|---|---|---|
matmul/small_generic_bf16_threads=8/1x256x256 |
30.24 µs | 37.52 µs | +24.1% | |
matmul/small_generic_bf16_threads=1/1x256x256 |
34.21 µs | 39.57 µs | +15.7% | |
| ✅ | matmul/large_generic_bf16_threads=1/32x1024x1024 |
1.97 ms | 2.22 ms | +12.5% |
| ✅ | sampling_latency/greedy_per_token |
3.47 µs | 3.76 µs | +8.6% |
| ✅ | grammar_masking/llguidance_compute_mask/32 |
77.84 µs | 83.15 µs | +6.8% |
| ✅ | matmul/medium_generic_bf16_threads=1/32x512x512 |
583.03 µs | 609.58 µs | +4.6% |
| ✅ | qwen3_sampling_processors/top_p_fast_after_top_k |
545.58 µs | 570.12 µs | +4.5% |
| ✅ | block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1 |
170.49 µs | 175.12 µs | +2.7% |
| ✅ | qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline |
3.77 ms | 3.86 ms | +2.4% |
| ✅ | qwen3_sampling_processors/top_k_top_p_fast |
684.48 µs | 696.00 µs | +1.7% |
| ✅ | matmul/large_generic_bf16_threads=8/32x1024x1024 |
1.71 ms | 1.71 ms | +0.3% |
| ✅ | matmul/small_generic_f32_threads=1/1x256x256 |
40.59 µs | 40.67 µs | +0.2% |
| ✅ | matmul/large_generic_f16_threads=1/32x1024x1024 |
86.71 µs | 86.45 µs | -0.3% |
| ✅ | add/medium_f16_threads=1-internal/262144 |
116.81 µs | 116.34 µs | -0.4% |
| ✅ | logit_processing/seven_processor_chain_per_step |
327.62 µs | 324.68 µs | -0.9% |
| ✅ | sampling_latency/top_k_per_token |
55.33 µs | 54.78 µs | -1.0% |
| ✅ | add/small_bf16_threads=1-internal/1024 |
490.2 ns | 483.0 ns | -1.5% |
| ✅ | matmul/medium_generic_bf16_threads=8/32x512x512 |
501.71 µs | 485.41 µs | -3.2% |
| ✅ | add/medium_bf16_threads=1-internal/262144 |
112.29 µs | 108.29 µs | -3.6% |
| ✅ | sampling_latency/top_p_per_token |
421.49 µs | 397.92 µs | -5.6% |
| ✅ | add/small_f32_threads=1-internal/1024 |
240.6 ns | 226.4 ns | -5.9% |
| ✅ | kv_cache/alloc_dealloc_pages |
41.39 µs | 38.81 µs | -6.2% |
| ✅ | tokenization/decode_tokens_per_second |
7.07 ms | 6.61 ms | -6.5% |
| ✅ | add/medium_f32_threads=1-internal/262144 |
28.59 µs | 26.62 µs | -6.9% |
| ✅ | gather/medium_f16_threads=1-internal/32768 |
2.67 µs | 2.48 µs | -7.0% |
| ✅ | qwen3_sampling_processors/top_k_full_sort_baseline |
2.29 ms | 2.12 ms | -7.1% |
| ✅ | qwen3_sampling_processors/top_k_partial_selection |
159.19 µs | 147.81 µs | -7.1% |
| ✅ | qwen3_sampling_processors/top_k_top_p_full_sort_baseline |
5.90 ms | 5.44 ms | -7.8% |
| ✅ | tokenization/encode_tokens_per_second |
435.15 µs | 398.77 µs | -8.4% |
| ✅ | matmul/small_generic_f32_threads=8/1x256x256 |
47.50 µs | 43.24 µs | -9.0% |
| ✅ | matmul/medium_generic_f16_threads=1/32x512x512 |
42.33 µs | 38.34 µs | -9.4% |
| ✅ | sampling_latency/min_p_per_token |
260.78 µs | 236.05 µs | -9.5% |
| ✅ | matmul/large_generic_f16_threads=8/32x1024x1024 |
96.48 µs | 87.25 µs | -9.6% |
| ✅ | matmul/small_generic_f16_threads=1/1x256x256 |
40.84 µs | 36.50 µs | -10.6% |
| ✅ | block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024 |
764.10 µs | 667.55 µs | -12.6% |
| ✅ | reduce_mean/large_f32_threads=1-internal/262144 |
1.27 ms | 1.11 ms | -12.7% |
| ✅ | gather/medium_f32_threads=1-internal/32768 |
4.78 µs | 4.12 µs | -13.6% |
| ✅ | add/small_f16_threads=1-internal/1024 |
559.4 ns | 478.8 ns | -14.4% |
| ✅ | matmul/medium_generic_f16_threads=8/32x512x512 |
39.42 µs | 33.53 µs | -14.9% |
| 🟢 | gather/medium_bf16_threads=1-internal/32768 |
2.84 µs | 2.41 µs | -15.1% |
| 🟢 | matmul/large_generic_f32_threads=1/32x1024x1024 |
11.94 ms | 10.13 ms | -15.2% |
| 🟢 | matmul/large_generic_f32_threads=8/32x1024x1024 |
5.75 ms | 4.76 ms | -17.2% |
| 🟢 | reduce_mean/medium_f32_threads=1-internal/65536 |
307.71 µs | 254.06 µs | -17.4% |
| 🟢 | gather/large_bf16_threads=1-internal/131072 |
20.36 µs | 16.22 µs | -20.3% |
| 🟢 | block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1 |
660.29 µs | 515.49 µs | -21.9% |
| 🟢 | matmul/medium_generic_f32_threads=1/32x512x512 |
3.07 ms | 2.37 ms | -22.8% |
| 🟢 | matmul/small_generic_f16_threads=8/1x256x256 |
46.56 µs | 35.77 µs | -23.2% |
| 🟢 | gather/large_f32_threads=1-internal/131072 |
60.33 µs | 45.04 µs | -25.4% |
| 🟢 | add/large_bf16_threads=1-internal/4194304 |
2.41 ms | 1.78 ms | -26.0% |
| 🟢 | reduce_mean/small_f32_threads=1-internal/4096 |
23.05 µs | 16.44 µs | -28.7% |
| 🟢 | gather/large_f16_threads=1-internal/131072 |
22.49 µs | 15.98 µs | -28.9% |
| 🟢 | add/large_f16_threads=1-internal/4194304 |
2.65 ms | 1.80 ms | -32.0% |
| 🟢 | gather/small_f32_threads=1-internal/4096 |
1.07 µs | 687.6 ns | -35.8% |
| 🟢 | block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024 |
80.16 µs | 49.74 µs | -38.0% |
| 🟢 | add/large_f32_threads=1-internal/4194304 |
1.28 ms | 779.06 µs | -39.3% |
| 🟢 | gather/small_bf16_threads=1-internal/4096 |
836.7 ns | 494.5 ns | -40.9% |
| 🟢 | matmul/medium_generic_f32_threads=8/32x512x512 |
1.84 ms | 1.04 ms | -43.8% |
| 🟢 | gather/small_f16_threads=1-internal/4096 |
905.9 ns | 507.7 ns | -44.0% |
| 🟢 | block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024 |
134.34 µs | 62.44 µs | -53.5% |
Visual flags:
Host info
CPU: Apple M1 (Virtual)
Cores: 3
OS: Darwin 25.5.0 arm64
Rust: rustc 1.97.1 (8bab26f4f 2026-07-14)
Load avg: { 5.36 4.04 7.29 }
What this cannot catch
- Regressions in code paths not covered by these benchmarks (e.g., end-to-end decode with a real model)
- Sub-threshold regressions that compound over multiple PRs
- Performance changes that only manifest under GPU execution
- Latency changes in the ORT integration path (these benchmarks exercise the native Rust kernels)
Owner
Author
Reviewer verification — reproduced, mergingValidated on Intel Core i7-13800H (14C/20T), Windows 11, base
Merging by squash on this evidence. |
justinchuby
added a commit
that referenced
this pull request
Aug 18, 2026
…to repay it (3.15x on small shapes) (#1149) ## What this changes `half_gemm::gemm_impl` forked **every** half GEMM across the rayon pool with no work guard, so it paid fork/join overhead to multiply tiny operands a single core finishes far faster. Since the f16 prefill gate (#1140) this kernel only receives the *small* shapes that decline widening, so the unguarded fork was mis-sized for essentially every shape it still serves. Two guards, scheduling-only (arithmetic is untouched): 1. `PARALLEL_MIN_WORK = 1_048_576` on `m*k*n` — stay serial below it. 2. Never fork when the split yields a **single block** (`m.div_ceil(split_mc) == 1`); it cannot use more than one thread, so it is pure overhead however large the operand. `m == 1` always lands here. ## Measurement (reviewer re-measurement) Re-measured by the squad reviewer on **Intel Core i7-13800H (14C/20T), Windows 11**, via the in-repo `bench_half_gemm_parallel_threshold` A/B harness (`m = 8`, serial vs pool-split interleaved rep-by-rep, p50 of 9), at `RAYON_NUM_THREADS = 4/8/16`. The last column is `serial/par` — **>1 means splitting wins**. Smallest shapes (the ones this guard sends serial) — splitting is catastrophic, so declining it is the win: | `m*k*n` | T=4 | T=8 | T=16 | |---|---|---|---| | 32,768 (8×64×64) | 0.08 | 0.10 | 0.09 | | 65,536 | 0.13 | 0.16 | 0.15 | | 131,072 | 0.21 | 0.31 | 0.23 | So the worst shape (8×64×64) goes from a forked path to serial and is **~10–12× faster** on this box (par 0.39/0.25/0.42 ms → serial 0.033/0.025/0.037 ms at T=4/8/16). That is even larger than the 3.15× the original body reported on its Linux reference box, because this laptop's relative fork/join overhead is higher. Around the threshold and above: | `m*k*n` | T=4 | T=8 | T=16 | |---|---|---|---| | 1,048,576 (threshold) | 0.98 | 0.68 | 0.76 | | 1,572,864 | 0.95 | 0.79 | 0.96 | | 2,097,152 | 0.91 | 1.19 | 1.01 | | 3,145,728 | 1.29 | 1.49 | 1.17 | **Honest hardware caveat.** On *this* box the crossover where splitting starts to pay sits **higher than `1_048_576` — around 2–3M MACs**, not at ~1M. The original constant was derived on a 16-core Linux box. This does **not** make it unsafe here: the guard only ever moves *below-threshold* work to serial, and leaves everything `>= PARALLEL_MIN_WORK` splitting exactly as `main` does today. So relative to the current unguarded base there is **zero regression at any shape** — every shape either improves (below threshold) or is byte-for-byte the same code path (at/above). The only imperfection on this box is that shapes in ~[1M, 2.5M] still fork and lose 0.68–0.98× as they already do on `main`; the guard leaves that (pre-existing) money on the table rather than creating a new regression. I deliberately did **not** retune the constant to this laptop: it is a defensible, evidence-backed value pinned by a test and an evidence table, and overfitting it to a single dev box would be worse than a conservative shared default. If the CPU-EP owner wants the constant re-derived on the reference hardware, that is a separate, non-blocking follow-up. ### No large-shape regression Explicitly checked, because the risk of a "don't fork below N" heuristic is that N is wrong for some *other* shape: every shape `>= PARALLEL_MIN_WORK` takes the identical split path as base (the guard's `parallel` branch is unchanged), so nothing large got slower. Confirmed above — the 1.5M–3.1M rows behave as they do on `main`. ## Correctness The guards change **scheduling only, never arithmetic**, so both routes must agree **bit-for-bit** (compared on `to_bits()`), for f16 and bf16. All pass on this box: - `both_routes_agree_bit_for_bit` / `bf16_routes_agree_bit_for_bit` — spans `m > MAX_MC` so the serial multi-block indexing is actually exercised. - `half_gemm_declines_to_split_work_below_the_crossover` — proves the route via a thread-local counter, both directions. - `the_crossover_itself_splits_and_one_mac_below_it_does_not` — exact boundary. - `a_single_block_is_never_split_however_large_the_operand`. - `half_gemm_parallel_threshold_matches_the_measured_crossover` — pins the constant. ## Validation (reviewer) - `cargo test -p onnx-runtime-ep-cpu --lib` — **1352 passed, 0 failed, 17 ignored** (stacked on #1183; the 6 new guard tests account for the delta). - `cargo clippy -p onnx-runtime-ep-cpu --all-targets -- -D warnings` — clean. ## Scope This does not move the `K=N=2048` plugin benchmarks (those shapes are ≥4.2M MACs, far above the threshold). The win is confined to half GEMMs below ~1M MACs, which is the range this kernel actually serves post-#1140. --------- Co-authored-by: Roy <roy@squad.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 18, 2026
… (1.66x -> 1.36x vs ORT) (#1218) ## What this changes `erf_ps`'s big-branch `R` polynomial and the `exp_ps` polynomial it feeds are switched from Horner's rule to **Estrin's scheme**. Horner is a six-deep dependent FMA chain; Estrin regroups it as `a^3·(c0 a^3 + c1 a^2 + c2 a + c3) + (c4 a^2 + c5 a + c6)`, three deep for two extra multiplies, whose halves the out-of-order engine runs in parallel. Only the two chains actually on `erf`'s critical path are converted; the small branch stays Horner (it runs in parallel and is off-path). Rebased onto current `main` so it **preserves #1227** (`map_ps`'s `#[target_feature(enable = "avx2,fma")]`). The original branch predated #1227 and its raw diff would have moved that attribute back onto `map_bias_ps`; that hunk is dropped. Only `simd_activations.rs`'s erf/exp polynomials and one accuracy test change here. ## Numerics — measured, not assumed (reviewer) Estrin reassociates f32 FMAs, so it is **not** bit-identical to Horner by construction. The squad reviewer measured the actual error with a two-build dump-and-diff (base Horner build vs this Estrin build, identical input grid of 404,045 points spanning `[-6,6]` densely, the `0.921875` split boundary, large `|x|`, denormals, and NaN/±inf), on **Intel Core i7-13800H, Windows 11**: - **Estrin vs Horner: max difference = exactly 1 ULP.** 4,401 of 404,043 finite points (1.09%) differ, every one of them by a single ULP; max absolute diff 5.96e-8. Nothing moves by more than one ULP anywhere in the domain. - **vs f64 `erf` truth:** max |Horner − erf| = 5.77e-8, max |Estrin − erf| = 6.55e-8 (worst at x≈0.933, just past the split). Both within ~1 ULP of the correctly rounded result; Estrin is at most ~0.8e-8 looser than Horner and never worse than 1 ULP from it. - **Special values are all correct and identical between the two:** large `|x|` (7,8,10,20,50,100,1000,1e30,f32::MAX) saturate to ±1.0; denormals produce bit-identical tiny outputs; NaN → NaN; +inf → +1.0; −inf → −1.0. This is a conscious, documented 1-ULP change, not a hand-wave. The accuracy gate `erf_reassociation_costs_no_accuracy` (worst scaled error pinned at 5.97e-8 = 2^-24 over `[-6,6]`) **passes** on this box, so a future regrouping that widens the envelope fails instead of being absorbed by the looser `ERF_BOUND`. ## Performance — measured on this box, ORT-independent The original body quoted "1.66× → 1.36× vs ORT" (≈ −17.7% kernel time). I could not reproduce that specific figure: no pinned ORT reference on this Windows box, and it is different hardware. Instead I timed `erf_avx2` directly, same machine, two builds, single-thread, min-of-200 × 9 rounds over a 1,048,576-element mixed-range buffer: | build | per-round min (ms) | best | | --- | --- | ---: | | Horner (base) | 0.6346–0.6348 (8/9 rounds) | 0.6346 | | **Estrin (this PR)** | **0.6042–0.6043 (8/9 rounds)** | **0.6042** | The ranges do not overlap, so the direction is solid: **Estrin is ~5.0% faster (1.05×)** on the erf kernel here. That is a real win but smaller than the −17.7% originally claimed — reported honestly as the measured kernel-internal delta rather than an unverified ORT ratio. The mechanism (a latency-bound chain freed by shorter dependency depth) is consistent with a positive but hardware-dependent magnitude. ## Validation (reviewer) - `cargo test -p onnx-runtime-ep-cpu --lib` — **1353 passed, 0 failed, 17 ignored** (base 1345 + #1183/#1149 + this test), including the accuracy gate. - `cargo clippy -p onnx-runtime-ep-cpu --all-targets -- -D warnings` — clean. - No MLAS feature involved; `map_ps`'s target_feature (#1227) verified intact in the merged result. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com> Co-authored-by: resch <resch@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 23, 2026
…d a knob that no longer exists (#1822) Follow-up to #1173, correcting two defects I shipped in it and repairing the rule they undermined. Docs, one ledger string, one new test, one new script. No production kernel or routing change. ## 1. The ledger named a route gate that had already been deleted `PLAN[MatMulF32].shape_gate` said the native `SimdX86` route "gates M=1 on `ONNX_GENAI_CPU_MM_SIMD_M1_GEMV` (default off, #1116)". #1183 shipped that GEMV on by default and removed the probe. `git merge-base --is-ancestor 5417d04 bdb4599` confirms it landed **before** #1173 merged — so the ledger was wrong the day it landed. Today `sgemm_simd` calls `sgemm_simd_variant(a, b, c, m, k, n, true)` unconditionally and `use_m1_gemv` is a plain parameter that only the in-process A/B harness passes as `false`. No environment variable reaches that route. `docs/performance/CPU_MATMUL_ASSIGNMENT.md:559` already recorded the correct fact ("It is measured now, and the route is the default. There is no env probe on the dispatch any more"). Two files in the same directory disagreed and nothing compared them. **Now guarded.** `ledger_prose_only_names_environment_variables_that_still_exist` requires every `NXRT_*` / `ONNX_GENAI_*` token in the ledger's prose to still exist as a string literal in the crate's sources. It cannot check that the description is *right*, only that the knob is *real* — which is the half that goes stale silently. Mutation-verified, not just observed green: ``` matmul_f32: ledger prose names environment variable `ONNX_GENAI_CPU_MM_SIMD_M1_GEMV`, but no source file in this crate contains the literal "ONNX_GENAI_CPU_MM_SIMD_M1_GEMV". ``` ## 2. The doc published a toggle A/B that could not have been run #1173 carried a table captioned **"same binary, same session, toggle the only difference"**, reporting `decode 1×2048×2048` at 0.146 with `ONNX_GENAI_CPU_MM_SIMD_M1_GEMV` off against 0.337 with it on, and called turning it on "the obvious next slice". Nothing reads that variable. Setting it measures the same route twice; it cannot produce two different columns. The table is withdrawn and the retraction kept in the text rather than quietly deleted. This is the failure mode the document's own graduation rule warns about — **an arm that was not on the route it was labelled with** — committed by the document that wrote the rule. It survived review because a plausible number in a well-formed table is not self-evidently unmeasured. Readers are pointed at `bench_f32_gemm_ab`, which holds the route as a function parameter and carries the M≥2 rows as a built-in control. ## 3. The gap table is re-measured and the ≥5% rule is repaired The old table was one unguarded invocation per row at an unstated width, taken before the decode-placement corrections (#1729, #1794, #1811) — i.e. when the decode pool put 16 workers on 8 physical cores. New harness: `scripts/bench_native_vs_mlas_width.py`. Arms interleaved rep by rep so host drift lands on both equally; per-rep `os.wait4` CPU-efficiency guard adapted from #1809; six reps per arm; two widths. Raw verdicts, spreads and discards are all reported rather than summarised away. **Three findings, all about method rather than kernels.** | | narrow (6 cores, 1 L3) | wide (32 logical CPUs) | |---|---|---| | `matmul_f32 16×512×512` | 1.581, spread 41% | 0.866, spread 134% | | `matmul_f32 decode 1×2048×2048` | 1.117, spread 21% | 0.934, spread 13% | - **Two cases change verdict on width alone.** Same binary, same half-hour, only the CPU mask differs. `x86_sgemm` parallelises over column strips and MLAS declines to parallelise some shapes, so interleaving the two *routes* inside one process does not protect the ratio — it changes both at once. - **`16×512×512` disagrees with itself on both arms**, alternating `keep-mlas` / `native-graduates` from a byte-identical binary. **One more run of the old table could have graduated a route on this row.** - **The narrow arm is more trustworthy despite having fewer cores** — spreads 4–42% against 5–134%, and it lost no reps to the guard. Isolation beat parallelism. **Softmax now decomposes cleanly**, because no vendored MLAS kernel has changed since #1173 (the only `mlas-sys` edits are the additive straggler handshake in `work_stealing_pool.rs`, #828/#1714, which adds waiting). At matched width the MLAS control arm is stationary to within 4% while native improved **1.24–1.27×** — matching #1416's claim for the row kernel. The f32 GEMM rows get no such attribution and now say so explicitly: their control moved **2.0× the wrong way**, so only the current ratio at a stated width is defensible. **The rule gains what it lacked**: spread must be smaller than the claimed win; reps that did not get the CPU are discarded rather than averaged; a verdict is valid only at a stated width. Under it, `decode 1×2048×2048` — the first f32 GEMM case to show a real native win — **still does not graduate**: it costs more CPU (cpu_ratio 0.875), does not hold at 32 threads, and its 21% spread exceeds its 12% win. ## The width claim is verified, not asserted #1815 landed while this was in progress and observed the neighbouring `bench_generic` harness spawning its ORT arm *outside* the affinity confinement it applied to the native arm. That hazard applies to any `taskset` claim, including mine, so I checked it instead of trusting it — sampling `Cpus_allowed_list` from `/proc/<pid>/task/*/status` 40× across a live narrow-arm run: ``` '16,20,22,26,28,30': 478 observations native_vs_mlas- 273, mlas-sys-ws-0..4 39 each, nxrt-task-0..4 2 each '0-31': 1 (the taskset process itself, before exec) ``` Both routes confined identically; no thread escaped. The rule now requires this check. ## Validation - `dispatch_ledger` **17/17**, including the new falsifier, after merging latest `main`. - `default_artifacts_are_mlas_free` **9/9** — the no-MLAS-in-defaults invariant is untouched. - `cargo clippy -p onnx-runtime-ep-cpu --lib --all-targets` clean; `cargo fmt --check` clean. - Normal merge of `origin/main` (`aee2b9d11`), no rebase, no conflicts. ## Limitations - The narrow arm is six cores on one L3 of one x86-64 host. Nothing here transfers to aarch64 or to a two-socket box. - The `activations erf 1 Mi` row shows native 13.5% slower at matched width. The nearest scatter figure is the wide arm's 8% spread, but that is a spread of *ratios* against a move in a *native time*, so the two are not strictly commensurable. Its MLAS control also moved 11%. **Flagged for pinned re-measurement, not reported as a regression.** - The wide arm was taken with ~4–5 cores of unrelated load present. That is stated in the doc rather than hidden, and it is why its spreads are wider; the guard reports which reps were discarded instead of pretending the host was quiet. - No production behaviour changes here, so there is no performance claim to make about the shipped artifact. Refs #1173, #1183, #1809, #1815, #1416. ## Independent review, and what it changed An independent adversarial review of the full diff returned **no blockers** — it confirmed the ancestry argument behind the retraction, the stationary-control premise for the softmax attribution, and that the headline case is correctly *refused* by the rule (21% spread against a 12% win). It also found seven real defects, all now fixed in `f0323f9ed`. The one that mattered most was in the new test. It only proved the variable name appeared *somewhere* in the crate, so a variable whose read site had been deleted but whose name survived in an `EnvVarGuard::set(...)` line would still have passed — which is the precise shape of the defect this PR exists to correct. The test now requires the matching line to be an `env::var(` / `env::var_os(` read or an `_ENV: &str =` binding. Verified by mutation in **both** directions: | mutation | before | after | |---|---|---| | reinsert retired `ONNX_GENAI_CPU_MM_SIMD_M1_GEMV` into ledger prose | fails ✅ | fails ✅ | | retire the two real `NXRT_CPU_GEMM_BACKEND` reads, leaving the literal only in test guards | **passes ❌** | fails ✅ | The remaining six were prose defects in the doc: a stated spread range that contradicted its own table's 82% row, "within 4%" against a table reading −4.2%, a narrow-arm ratio fused with a wide-arm attribution, a spread quoted as 7.5% that was 8% *and* compared against an incommensurable quantity, the CPU-efficiency guard oversold as "what makes this table measurable at all" (in-process interleaving is what protects the ratio; the guard catches only *differential* descheduling), and a one-directional provenance argument standing in for the direct control measurement that actually carries the softmax attribution. **Two further defects I found myself while checking the tables against each other**, neither raised by the review: - The `ratio` column is a median of per-rep ratios while the `ns/unit` columns are medians of times. Medians do not distribute over division, so every row looked internally inconsistent to anyone who tried to divide it out (`0.0684 / 0.0617 = 1.109` against a stated `1.117`). Now documented, along with why the per-rep form is the correct one to quote: it pairs each MLAS invocation with the native invocation it was interleaved against, which is the entire point of interleaving. The then→now figures are relabelled as quotients of medians. - "wider than nine of the twelve wide-arm rows" was eleven of twelve. ## Adopting #1814 `aee2b9d11` (#1814) landed on `main` while this was in review, and it closes the exact hole the review found in the guard this document recommends. A differential CPU-efficiency check cannot see contention that lands evenly on both arms; #1814's confined-set meter reads busy jiffies on the process's own `Cpus_allowed_list` and subtracts the process's own CPU, so foreign load shows up directly. The rule now points at it, and the tables here are explicitly marked as predating it and guarded by the weaker method. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
What this changes
#1091 added a native
M == 1f32 SGEMV to the SIMD SGEMM but gated it behindONNX_GENAI_CPU_MM_SIMD_M1_GEMV, defaulting off. The shipped build thereforenever executed it:
MatMulf32 atM = 1— every decode step — kept running thepacked prefill kernel, which packs a one-row panel, walks it once and discards it.
This makes the
m == 1GEMV route unconditional insgemm_simd, deletes the envprobe and the
m1_gemv_enabled()helper.use_m1_gemvsurvives only as a parameterof
sgemm_simd_variant, which is how thebench_f32_gemm_abA/B harness drives botharms in one process. Production no longer pays a locked
getenvper GEMM call.Measurement (reviewer re-measurement)
Measured by the squad reviewer on Intel Core i7-13800H (14C/20T), Windows 11,
single-threaded (
RAYON_NUM_THREADS=1), via the in-repobench_f32_gemm_absame-binary A/B harness (
simd_gemvvssimd_packed, min-of-N timing). Thisharness drives exactly the two routes the default flip chooses between, so the
packed/gemvratio is the speedup this PR unlocks in production. It isORT-independent; see the caveat below on the "vs ORT" figure.
packed/gemv(higher = the newly-default GEMV is faster), M=1 decode shapes,across 4 runs:
Control (M=128 prefill, byte-identical code both arms):
gemv/packedranged0.97–1.05 across all runs — i.e. ≈1.0 as required, worst drift ~4.6% on this shared
box. The unchanged-code rows not moving is the honesty check that the M=1 movement
is real and not machine noise.
The win is a robust ~5× (shape-dependent, 4.7–9.9×) at M=1, reproduced over four
interleaved runs. The route is reached only for
m == 1; everym >= 2shapegenerates byte-identical code before and after (the control).
On the "vs ORT" number
The original body quoted "6.85x → 1.15x vs ORT". I did not reproduce that
specific ratio: I have no pinned ORT f32 reference build on this Windows box, and a
ratio against an unnamed ORT build is exactly the kind of figure this team has
withdrawn before. What I can stand behind is the packed→gemv delta above (~5×),
which is consistent with the packed path being several× slower than ORT and the GEMV
landing near parity. The headline is stated as the measured internal speedup, not an
unverified ORT multiplier.
Numerics
f32 addition is not associative, so the GEMV changes M=1 results by reassociation
versus the packed path. This is bounded by the test tolerance
(
1e-3 * (1 + max_ref)) and is deterministic: the GEMV does no horizontal reduction(each output column is a lane reduced across
kin a fixed unroll-by-4 order) andevery strip start is a multiple of 8, so the same input yields the same bits at any
thread count.
m >= 2is unaffected (byte-identical).Tests
the_default_entry_point_routes_m1_to_the_gemv— assertssgemm_simd(m=1)equalssgemm_simd_m1bit-for-bit, withm=2equal to the packed path as a control.Reverting the default to a probe fails it.
m1_route_matches_packed_within_tolerance— now compares against the genuinepacked path (
sgemm_simd_variant(.., false)) with anassert_ne!proving the twoarms are different code, so the tolerance check is non-vacuous.
env::set_vartest from the original was correctly dropped (data race againstparallel
env::varreaders; also couldn't fail for its own regression).Validation (reviewer)
cargo test -p onnx-runtime-ep-cpu --lib— 1346 passed, 0 failed, 16 ignored(base 1345 + 1 new route test) on base
8a90878a.cargo clippy -p onnx-runtime-ep-cpu --all-targets -- -D warnings— clean.mlasarm of the harness is areference arm only and was not used for the numbers above.