From 1d23f9f42d765b0cff34d1b3925698cd33dacd15 Mon Sep 17 00:00:00 2001 From: Justin Chu Date: Tue, 25 Aug 2026 08:39:00 +0000 Subject: [PATCH] test(ep-cpu): assert the fast-path property, not one platform's route to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `real_unwindowed_overlapping_frames_match_independent_reference` reads `DFT_FFT_TEST_HITS` — the radix-2 counter — and requires it to advance by three. On Apple targets that assertion is false by construction, not flaky: `DftPlan::new` builds a vDSP setup for every power-of-two n >= 4, and `transform` returns from the vDSP branch before reaching the radix-2 one, so a frame length of 4 increments `DFT_VDSP_TEST_HITS` and leaves `DFT_FFT_TEST_HITS` at zero. The test therefore asserts that macOS did not use macOS's fast path. It reddens `Rust coverage (macOS arm64)` on every branch cut after #2083. `fft_fallback_reachability` in dft.rs survives only because it uses n = 2, below the vDSP minimum of 4. The property the test means to check is shared by every target: a power-of-two frame must not fall back to `naive_dft_into`. `dft:: fast_path_hits()` reports exactly that, summing whichever counters this target's fast paths increment, so the assertion no longer names a route. Falsified in both directions on Linux, since no macOS target compiles here (aarch64-apple-darwin: E0463, no std in this toolchain): - forcing the power-of-two branch to `naive_dft_into`: the test FAILS with the fix in place, so it is not vacuous; - emulating Apple's dispatch rule (pow2 && n >= 4 -> vDSP counter, early return): passes with the fix, and with the pre-fix assertion restored reproduces the macOS CI panic on Linux. cargo test --locked -p onnx-runtime-ep-cpu --lib: 1796 passed, 0 failed. fmt and clippy --all-targets -D warnings clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/onnx-runtime-ep-cpu/src/kernels/dft.rs | 23 +++++++++++++++++++ .../onnx-runtime-ep-cpu/src/kernels/stft.rs | 10 ++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/onnx-runtime-ep-cpu/src/kernels/dft.rs b/crates/onnx-runtime-ep-cpu/src/kernels/dft.rs index 20a32e2ade..bcde7cb260 100644 --- a/crates/onnx-runtime-ep-cpu/src/kernels/dft.rs +++ b/crates/onnx-runtime-ep-cpu/src/kernels/dft.rs @@ -22,6 +22,29 @@ pub static DFT_VDSP_TEST_HITS: AtomicU64 = AtomicU64::new(0); /// Dispatch counter for the radix-2 FFT fallback path. pub static DFT_FFT_TEST_HITS: AtomicU64 = AtomicU64::new(0); +/// Power-of-two transforms that took *a* fast path — whichever one this target +/// has. +/// +/// `DFT_FFT_TEST_HITS` alone does not answer that question. On Apple targets +/// `DftPlan::new` builds a vDSP setup for every power-of-two `n >= 4`, and +/// `transform` returns from that branch before the radix-2 one, so a caller +/// asserting on the radix-2 counter there is asserting that the platform's own +/// fast path was *not* used — false by construction, and nothing to do with the +/// property it meant to check. +/// +/// What every target shares is that a power-of-two transform must not fall back +/// to `naive_dft_into`. That is what this sums, so a caller can assert the +/// property instead of one platform's route to it. +pub fn fast_path_hits() -> u64 { + let radix2 = DFT_FFT_TEST_HITS.load(Ordering::Relaxed); + #[cfg(any(target_os = "macos", target_os = "ios"))] + { + return radix2 + DFT_VDSP_TEST_HITS.load(Ordering::Relaxed); + } + #[cfg(not(any(target_os = "macos", target_os = "ios")))] + radix2 +} + pub struct DftFactory; impl KernelFactory for DftFactory { diff --git a/crates/onnx-runtime-ep-cpu/src/kernels/stft.rs b/crates/onnx-runtime-ep-cpu/src/kernels/stft.rs index 691c702493..2ab2506c18 100644 --- a/crates/onnx-runtime-ep-cpu/src/kernels/stft.rs +++ b/crates/onnx-runtime-ep-cpu/src/kernels/stft.rs @@ -263,11 +263,10 @@ fn positive_scalar(name: &str, input: &TensorView<'_>) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::kernels::dft::DFT_FFT_TEST_HITS; + use crate::kernels::dft::fast_path_hits; use crate::kernels::testutil::Owned; use onnx_runtime_ep_api::TensorView; use onnx_runtime_ir::{Attribute, NodeId}; - use std::sync::atomic::Ordering; fn node(onesided: i64) -> Node { let mut node = Node::new(NodeId(0), "STFT", vec![], vec![]); @@ -353,16 +352,17 @@ mod tests { let signal = Owned::f32(&[1, 8, 1], &values); let step = Owned::i64(&[], &[2]); let length = Owned::i64(&[], &[4]); - let before = DFT_FFT_TEST_HITS.load(Ordering::Relaxed); + let before = fast_path_hits(); let output = execute(&signal, &step, None, Some(&length), 0, &[1, 3, 4, 2]).unwrap(); - let after = DFT_FFT_TEST_HITS.load(Ordering::Relaxed); + let after = fast_path_hits(); let input: Vec = values.iter().map(|&value| value as f64).collect(); assert_close(&output.to_f32(), &reference(&input, 1, 2, 4, None, false)); assert_eq!(output.shape[1], 3, "the last eligible frame must be kept"); assert!( after >= before + 3, - "each power-of-two frame must use the radix-2 FFT path" + "each power-of-two frame must take a fast path rather than the naive DFT \ + (radix-2 everywhere, vDSP on Apple targets); before={before} after={after}" ); // The middle frame starts at sample 2. A non-overlapping increment // would instead transform samples 4..8 and fail this comparison.