Repository navigation
perf(cpu-ep): fuse the f16/bf16 activation narrow into the kernel pass - #1376
Merged
Merged
Conversation
The 16-bit activation path was a three-pass sandwich: widen the input into an f32 scratch the size of the whole tensor, run the kernel over all of it, then narrow all of it back. On a 1 Mi tensor that streams a 4 MB buffer to memory and reads it back for no reason -- the intermediate is dead the moment the narrow has seen it. Fuse the last two passes. When the output is a contiguous f16/bf16 tensor and the closure declares no extra reads, evaluate the kernel 8192 elements at a time into a 32 KiB scratch and narrow each chunk straight into the output, so the intermediate never leaves L1. Chunking is only sound for a pure elementwise map, and the gate for that is structural rather than a convention: the row-broadcast callers (BiasGelu, FastGelu with a bias) have to declare their bias through `also_read` so the direct-write arm can reject an aliasing output, and a non-empty `also_read` is exactly what keeps them on the whole-tensor path. Measured, 1 Mi, 1 thread, 60 iters, ours/ORT p50 (lower is better): Tanh f16 1.408 -> 1.213 (0.752 -> 0.653 ms, -13.2%) Four rounds landed on 1.217/1.214/1.214/1.212 with the ORT arm moving 1.0%, so the effect is an order of magnitude clear of the control. Exp f16 does not move because `Exp` never reaches `write_mapped` -- only Tanh, Sqrt and Erf route through the f32 path today -- which is the next thing to pull. Values are bit-identical to the unfused arm, and the new tests pin that across a length spanning several chunks and ending on a partial one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review caught a real divergence in the fused arm, and the first version of the test could not see it. `dispatch!` picks scalar or vector from the length of the slice it is handed, and for the approximated kernels the two are not bit-identical. Chunking with a plain `chunks(NARROW_CHUNK)` gives the last chunk whatever is left over, so a tensor of `NARROW_CHUNK + 16` elements ran its final 16 through the scalar path where a single whole-tensor pass would have run them vectorised. That is a silent change in emitted f16/bf16 bits versus main for every size with `numel % 8192` in `[8, 32)` -- and this module treats bit reproducibility as a correctness property, not a tolerance, because bf16 crosses to the CUDA EP. `chunk_end` now absorbs a trailing remainder shorter than `SIMD_MIN_LEN` into the chunk before it, and keeps every other boundary on an 8-wide lane so the vector run lands exactly where one pass would have put it. The original test used a 137-element tail, which is above `SIMD_MIN_LEN` and so structurally could not catch this. `a_short_tail_does_not_fall_off_the _vector_path` replaces that confidence with a witness it finds at run time: it searches for an input where the scalar and vector paths disagree *after* narrowing to f16 -- most f32 disagreements are washed out by the narrow, so checking in the f32 domain would have proved nothing -- and asserts the test found one. Reverting `chunk_end` to the naive form fails it at index 8192 with f16 bits 5146 vs 5145. Also state the real disjointness argument for the `unsafe` slice: a widened f16/bf16 input is always a fresh allocation, so it cannot alias the narrower output. Access ordering would not have made overlapping `&[f32]`/`&mut [u16]` sound. A `debug_assert` is the tripwire for a future caller that breaks it. Tanh f16 1 Mi is unchanged by the fix: 1.212 / 1.214 / 1.214. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔴 Benchmark Regression DetectedComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
The fused widen/compute/narrow arm is behind #[cfg(x86)], but NarrowFn, chunk_end, NARROW_CHUNK and the four tests that exercise chunk boundaries were not. An aarch64 build warned on all of them, and CI builds with -D warnings, so this would have failed there while a local x86 clippy run stayed green. All four tests assert properties of the chunked arm, which does not exist off x86, so gating them loses no coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1376 +/- ##
===========================================
- Coverage 82.60% 80.39% -2.21%
===========================================
Files 12 378 +366
Lines 5472 167927 +162455
Branches 5472 167927 +162455
===========================================
+ Hits 4520 135010 +130490
- Misses 759 28072 +27313
- Partials 193 4845 +4652
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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
The f16/bf16 activation path was a three-pass sandwich: widen the input into an
f32scratch the size of the whole tensor, run the kernel over all of it,then narrow all of it back. On a 1 Mi tensor that is a 4 MB buffer streamed out
to memory and read back for no reason — the intermediate is dead the moment the
narrow has seen it.
This fuses the last two passes. When the output is a contiguous f16/bf16 tensor
and the closure declares no extra reads,
write_mapped_readingevaluates thekernel ~8192 elements at a time into a 32 KiB scratch and narrows each chunk
straight into the output, so the intermediate never leaves L1.
The code already carried the diagnosis and named this as the fix:
This is that change, measured on its own.
Why the chunking is bit-identical
Two separate things have to hold, and the second one is subtle enough that the
first version of this PR got it wrong (see the review section).
1. The closure must be a pure elementwise map. A closure that reads across
lanes would see a different window per chunk. The gate is structural rather than
a convention:
BiasGelu,FastGeluwith a bias) mustdeclare their bias range through
also_read, because that is what letsoutput_direct_write_eligiblereject an aliasing output;also_readis exactly what keeps them on the whole-tensor path.A caller cannot reach the chunked arm without having already asserted it reads
nothing but
x. All fivewrite_mapped(emptyalso_read) call sites werechecked and are plain elementwise maps:
Gelu×2,QuickGelu,FastGeluwithout bias, and the
Tanh/Sqrt/Erfarm of unary elementwise.2. No chunk may be shorter than
SIMD_MIN_LEN.dispatch!picks scalar orvector from the length of the slice it is handed, and for the approximated
kernels the two are not bit-identical. A plain
chunks(NARROW_CHUNK)handsthe last chunk whatever is left over, so a tensor of
NARROW_CHUNK + 16elements would run its final 16 scalar where one pass runs them vectorised.
chunk_endabsorbs a trailing remainder belowSIMD_MIN_LENinto the chunkbefore it, and keeps every other boundary on an 8-wide lane so the vector run
lands where one pass would have put it.
This matters because the module treats bit reproducibility as a correctness
property rather than a tolerance — see the
host_chunk_lendocs and thethread-invariance tests — and bf16 results cross to the CUDA EP.
unsafeThe
&mut [u16]is built undervalidate(),is_contiguous(),numel() == n,and a dtype match on
Float16/BFloat16(which is what makes the element width16 bits). Disjointness from
xdoes not rest on access ordering — thatwould not make overlapping
&[f32]/&mut [u16]sound anyway. It rests onto_dense_f32_widenalways allocating a fresh buffer when it widens, so a16-bit output cannot be aliased by its own widened input. A
debug_assertis the tripwire for a future caller that breaks that.
Measurement
Production A/B,
plugin_path_ab_vs_plain_ort, 1 Mi elements, 1 thread,60 iterations, box gated to ≥93% idle. Ratio is ours/ORT, lower is better.
bench_tanh_f16_1mbench_exp_f16_1mRepeatability of the headline case — seven independent rounds, the last three
taken after the correctness fix, which did not move it:
The ORT arm moves 1.0% across the whole set against a 13.2% effect, so the
result is an order of magnitude clear of its own control.
Expdoes not move because it never reacheswrite_mapped— onlyTanh,SqrtandErfroute through the f32 path today. Widening that routing is theobvious follow-up and is deliberately not in this PR.
Small tensors are unaffected: the scratch is
(NARROW_CHUNK + SIMD_MIN_LEN).min(n),so for any
nbelow a chunk it allocates exactly the samenfloats the oldpath did.
Context: why f16 was the thing to fix
A full 1 Mi sweep of the activation family at 1 thread showed the native kernels
are already ahead of ORT on 17 of 18 f32 cases —
mish0.101,celu0.135,elu0.137,selu0.145,log0.203,sqrt0.309,erf0.428,tanh0.444,sigmoid0.451 — withrelu(1.026) the only exception, and that one isalready addressed by #1355. The two 16-bit cases were the real remaining
kernel-side losses, which is what pointed here.
Tests
a_short_tail_does_not_fall_off_the_vector_path— the one that guards theseam. It searches at run time for an input where the scalar and vector
paths disagree after narrowing to f16 (most f32 disagreements are washed out
by the narrow, so checking in the f32 domain proves nothing), asserts a
witness was found, and then pins fused == unfused for tails of 1, 8, 16 and 31.
chunk_end_never_emits_a_short_trailing_chunk— pins the decompositionitself across ~250 tensor sizes around chunk boundaries: every chunk is either
≥
SIMD_MIN_LENor the whole tensor, every interior boundary is lane-aligned,and no chunk overruns the scratch.
fused_narrow_is_bit_identical_across_chunk_boundaries— f16 and bf16against a single-shot reference across
Tanh/Erf/Sigmoidfor seven taillengths.
a_row_broadcast_closure_stays_off_the_chunked_arm— aBiasGeluwith rowwidth 61 (never divides a chunk) over a multi-chunk tensor.
Every one was verified non-vacuous by mutating production code and observing the
failure. Reverting
chunk_endto the naive form fails the seam test at index8192 with f16 bits 5146 vs 5145.
Suites, on this branch:
onnx-runtime-ep-cpu --lib, debug profile: 1451 passed, 0 failedplugin_ort_e2ewithNXRT_REQUIRE_ORT_TESTS=1: 55 passed, 0 failed,including
every_assigned_node_is_also_executed_by_this_ep— assigned ==executed conformance holds.
cargo clippy -p onnx-runtime-ep-cpu --lib --all-targets: clean.cargo fmt -p onnx-runtime-ep-cpu -- --check: clean.Review
Independently reviewed by Claude Opus 4.8 with a brief to attack the safety
argument and verify every test by mutation. Verdict: REQUEST CHANGES, one
blocker, which is the
SIMD_MIN_LENtail divergence described above. Thereviewer reproduced it with a
Sigmoidf16 tensor of 8208 elements (bits 3455vs 3454) and correctly observed that the original 137-element tail could not
catch it. Fixed in
887c22e51, along with the reviewer's point that thealiasing justification was argued from access ordering rather than from the
allocation that actually guarantees it.
The reviewer also confirmed: all eight call sites audited and the structural
also_readgate holds; theFnOnce→Fnrelaxation is safe (every closurecaptures only
Copystate, noCell/RNG/lazy-init); the guard set matcheswrite_dense_f32_narrow;n == 0,numel() != nand non-x86 all fall throughunchanged; and small-
nallocation is not a regression.Scope
No ORT CPU fallback, no runtime or default MLAS — pure native x86, one function
plus its tests.
Re-validated on latest
main(e0aedd0fa), 2026-08-19Latest
mainmerged in normally (no rebase). Re-measured paired againstmainfrom the same box, 1 thread pinned, 60 iters / 10 warmup,
ours_p50 / ort_p50:mainRatio 1.367 → 1.218 (median), our own time −11%. ORT drifted under 1%
between the arms (0.534–0.541 on
main, 0.535–0.539 here), which is thecontrol. The distributions do not overlap.
Cross-platform fix (
f9a97e853)The fused arm is behind
#[cfg(x86)], butNarrowFn,chunk_end,NARROW_CHUNK, the test helperscalar_vector_divergenceand the four teststhat exercise chunk boundaries were not. An aarch64 build warned on every
one of them, and CI builds with
-D warnings— so this branch would havefailed cross-platform CI while a local x86
clippyrun stayed green.Now gated alongside the code they belong to: 0 warnings on both x86-64 and
aarch64. All four tests assert properties of the chunked arm, which does not
exist off x86, so gating them loses no coverage.
Validation on the merged base
cargo fmtclean;cargo clippy --all-targets -D warningscleanonnx-runtime-ep-cputests, debug profile (debug_asserts live)NXRT_REQUIRE_ORT_TESTS=1, release)every_assigned_node_is_also_executed_by_this_epandevery_fixture_loads_with_cpu_fallback_disabledgreen — nothing defers,nothing falls back to the ORT CPU EP
--all-targetscheck clean, 0 warnings