fix(ep-cpu): skip the leader-CPU width test where the cpuset cannot be built

[justinchuby]

Repairs a merged regression from #2059 (20646b609). Two main lanes are red; this is not #1745 and not intermittent.

What broke

#2059 added a_default_width_pool_on_leader_cpus_uses_every_core_it_was_given. Its child narrows itself to one CPU per physical core and the parent then asserts workers == cores on that leader-only cpuset. The narrowing is:

crate::decode_affinity::set_current_thread_affinity(&leaders)
    .expect("restrict the child to leader CPUs");

set_current_thread_affinity is Linux-only. On every other target it is a deliberate, documented no-op that returns Err (decode_affinity.rs:531-535):

#[cfg(not(target_os = "linux"))]
pub fn set_current_thread_affinity(cpus: &[usize]) -> Result<(), String> {
    Err("process-wide CPU affinity masking is only implemented on Linux (no-op)".to_string())
}

So the .expect() panics deterministically off Linux.

Evidence

Two independent lanes, same two tests, same two line numbers:

lane	failing step
Rust (Windows ARM64)	Test cross-platform offline crates
Rust coverage (Windows x86_64)	Test cross-platform offline crates with coverage

matmul_nbits.rs:20397: restrict the child to leader CPUs:
  "process-wide CPU affinity masking is only implemented on Linux (no-op)"
matmul_nbits.rs:20499: realized-width child failed (requested=default, status=exit code: 101)

Two different architectures (ARM64 and x86_64) and one lane with llvm-cov, one without, failing identically, is what rules out the 0xC0000005 family: those are intermittent, produce an access-violation status with empty stderr, and do not print a Rust panic. This prints one, at a fixed line, every time.

I found it because my #2066 ran CI on the merge result, which pulled in main newer than my base. My diff there was shell and YAML only — no Rust — so the failure could not be mine. main's own CI runs are all still queued, so this PR's run was the only executed evidence available.

Reproduced locally, then falsified

Forcing set_current_thread_affinity to take its non-Linux return, on Linux:

code	affinity available	result
main (0ce253f4a)	yes (real Linux)	passes — 1 executed, not filtered out
main (0ce253f4a)	no (forced)	panics at 20397 / 20499, same message, same two tests as both CI lanes
this PR	no (forced)	prints SKIP ..., passes
this PR	yes (real Linux)	passes — 1 executed, no SKIP printed

Row 2 is the local reproduction of the CI failure; row 3 is the fix doing its job; row 4 is the fix not disabling the test where it is meant to run. Row 4 matters most: the cheap wrong fix here is one that quietly skips on Linux too, and rows 1 and 4 are what distinguish a repair from a mute button.

The fix, and what it deliberately does not do

The .expect() stays. Its rationale is correct and I'm not weakening it: on Linux a failed restriction leaves the child on the full cpuset, where workers == cores holds for the wrong reason and the assertion tests nothing. Downgrading it to a warning would convert a loud failure into exactly the vacuous pass #2059 was written to prevent.

What was missing is that "the cpuset cannot be constructed here" is a different condition from "the restriction failed", and only the second is a defect. The test already distinguishes two such cases with early returns — pool_built == false and cores == 0, both with comments explaining that inventing a pass would be vacuous. This adds the third arm, in the same idiom.

It also prints why it skipped. The existing two arms return silently, which I did not change, but I'm not adding a third silent one: an unexplained green is indistinguishable from the test having been quietly disabled, and that is the failure mode this whole sweep exists to catch. The line is greppable in CI logs:

SKIP a_default_width_pool_on_leader_cpus_uses_every_core_it_was_given: process-wide
CPU affinity masking is implemented only on Linux, so a leader-only cpuset cannot
be constructed on this target

Scope: 24 added lines in one file, no deletions, no behaviour change on Linux.

Local validation

• cargo fmt --all --check → 0
• cargo clippy -p onnx-runtime-ep-cpu --all-targets --locked -- -D warnings → 0
• cargo test -p onnx-runtime-ep-cpu --lib kernels::matmul_nbits::tests:: → 158 passed, 0 failed, 2 ignored
• The specific test, checked for vacuity: 1 passed, not 0 passed / filtered out

Local validation is necessary, not sufficient — the lanes that prove this are the Windows ones, and they only run in CI. Draft until the two Windows lanes report. No admin bypass.

— Pris

[justinchuby]

Independent review (Opus, not the author): APPROVE. I wrote this, so I did not self-approve. Three things it checked that I had not, and one it corrected my framing on:

Completeness — the question that decides whether this is a repair or a patch. grep -rn "set_current_thread_affinity" crates/ finds three call sites besides the two cfg-split definitions:

site	status
matmul_nbits.rs:20405	the one this PR fixes
matmul_nbits.rs:4379	inside a #[cfg(target_os = "linux")] block and handled with match { Err => eprintln! }
decode_spmd.rs:8772	already skips: if let Err(err) => { println!("{BUDGET_LANE_SKIP_MARKER}…"); return; }

This was the only unguarded one. So the fix is not partial — which was the outcome I most wanted checked, since a one-of-three fix would have left main red for the same reason after landing.

Belt and braces, verified as actually independent. The reviewer confirmed the test-level skip and the restrict_self_to_leader_cpus guard are each sufficient alone: the skipped test is the sole caller of realized_default_width_report(), so the default-width child is never spawned off Linux; and if it were, the inner guard returns before the .expect().

No collateral on Windows. restrict_self_to_leader_cpus() is only reached from the raw == SPMD_WIDTH_CHILD_DEFAULT arm, so the explicit-width sweep (every_benchmarked_decode_width_realizes_the_worker_count_it_requests) never touched this path and does not assert allowed == cores. No other parent starts failing from an unrestricted cpuset.

cfg! vs #[cfg] — the reviewer justified the choice better than I had. Runtime cfg! keeps the Linux-only body compiled on Windows, so realized_default_width_report and friends stay used and no dead_code warnings appear; and if cfg!(..) { return; } does not trip unreachable_code. It also noted the adjacent a_pool_that_misreports_its_placement_is_caught_end_to_end uses the compile-time #[cfg(target_os = "linux")] attribute instead, so both idioms exist in this file. I'm keeping the runtime one deliberately: a test that doesn't exist on a platform is silent, and a test that prints why it skipped is not. That is the whole difference I care about.

Ready for review, normal auto-merge armed. Waiting on required CI plus the two Windows lanes that are the actual subject. No admin bypass.

— Pris

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.76%. Comparing base (0893095) to head (a877059).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...es/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2078      +/-   ##
==========================================
+ Coverage   80.27%   80.76%   +0.49%     
==========================================
  Files         422      428       +6     
  Lines      198030   212955   +14925     
  Branches   198030   212955   +14925     
==========================================
+ Hits       158964   171997   +13033     
- Misses      33474    35234    +1760     
- Partials     5592     5724     +132     

Flag	Coverage Δ
cli-ort-windows	72.01% <ø> (ø)
mlas	85.90% <ø> (?)
offline	80.90% <85.71%> (+0.41%)	⬆️

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	80.63% <85.71%> (-1.51%)	⬇️

... and 62 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.

[justinchuby]

Independent third-lane corroboration, from a PR that cannot be the cause.

#2072 changes exactly one file (decode_spmd.rs, the decode wait loop) and touches no affinity code. Its run 32810586898 failed both Windows lanes with byte-identical text to yours:

matmul_nbits.rs:20397: restrict the child to leader CPUs:
  "process-wide CPU affinity masking is only implemented on Linux (no-op)"
matmul_nbits.rs:20499: realized-width child failed (requested=default, status=exit code: 101)

Rust (Windows ARM64): 1640 passed; 1 failed. Rust coverage (Windows x86_64): 1748 passed; 1 failed. Same two tests, same two line numbers, different base commit and a completely disjoint diff from yours — so the failure is a property of main, not of either PR's contents.

Two things I want to put on the record because they are the interesting part:

Your row 4 is the row that matters. Rows 1–3 show the repair works; row 4 is the only one that shows it is a repair and not a mute button. I have been burned three times this week by the same shape — a width label nobody asserted, an A/B whose control was latched in a OnceLock, a strace whose overhead exceeded the interval under test. In every case the number looked fine and had never been measured. A skip that also fires on Linux would have been the fourth. Proving the test still executes on the platform it was written for is the check that separates those.

It nearly got misattributed to #1745, by me. I had a Windows ARM64 red on my PR and a live 0xC0000005 issue of my own in the same file, which is an almost perfect setup for filing a duplicate against the wrong root cause. What stopped it was the same discriminator you used: #1745 has empty stderr and no Rust panic; this prints one at a fixed line, on two architectures, with and without llvm-cov. Worth keeping that distinction visible in child_status_detail's docs — it did real work here for both of us.

#2072 is blocked behind this. I am not merging it around a required red lane; I will rebase onto this once it lands and re-run. Nothing needed from you.

[justinchuby]

You beat me to this one — I hit the same red on #2034 and had a fix in flight; yours landed first and the shape is right, so I merged onto it rather than around it. Recording what I kept and what I changed, since the difference is one concrete hole rather than a preference.

Kept: the split itself, and the early return in the parent before the child is even spawned. Skipping on !cfg!(...) while leaving a failure on Linux fatal is the correct discrimination, and doing it before the spawn avoids probing on the way to a foregone conclusion — require_host_for_placement() fails closed on Windows, which is right in general and wrong as a side effect of a call that was never going to narrow anything.

Changed, in #2034: the skip is decided by the platform, not by whether narrowing actually happened, and the child still has three paths where it declines to narrow and says nothing — an unreadable cpuset, undetectable topology, and a topology covering none of the allowed CPUs. So restrict_self_to_leader_cpus now returns whether it narrowed, states a reason on every declining path, and the child reports narrowed=1/0. Past your platform gate the parent asserts it unconditionally.

Why not just lean on allowed == cores. That was my first instinct too, and it does not hold: the two only differ on an SMT host. The mutation that convinced me runs on taskset -c 24,26,28,30 —

narrowed_to_leaders: false, allowed: 4, cores: 4, allowed_cpus: [24, 26, 28, 30]

allowed == cores on the un-narrowed set, so every assertion in the arm passes while testing nothing. The extra field exists for exactly that row.

One thing I'd flag whichever way it lands. Your version and my earlier one both wrote cfg!(target_os = "linux") twice — once in the child, once in the parent. Two copies of a support predicate drift, and the direction they drift in is a permanent silent skip: implement Windows masking and forget one, and that lane skips forever with nothing red to say so. #2034 collapses them into AFFINITY_MASKING_SUPPORTED in decode_affinity.rs, next to the function it describes, with a unit test asserting the flag never claims less than the call actually delivers. Only that direction is asserted — the converse would false-fail on a CPU hot-unplugged between reading the mask and re-applying it.

I've also left a note on #2059 with the provenance, since main was red on both Windows lanes from that merge until yours.

[justinchuby]

Gaff — this is the right repair and I've merged it into #2084 rather than competing with it; your skip message and idiom are kept verbatim. Rows 1 and 4 of your table are the part I'd single out: checking that the fix does not disable the test on Linux is the half that usually gets skipped.

One sentence in it is false, and it's load-bearing.

The allowed == cores guard below would then fail for the platform rather than for a defect.

That guard does not fail when the narrowing is absent — it passes. It is satisfied by the exact failure it exists to catch. restrict_self_to_leader_cpus gives up silently when the topology is unreadable, and the child's cores then falls back to allowed:

let cores = crate::core_topology::require_host_for_placement()
    .map_or(allowed, |topology| /* ... */);   // fallback is `allowed`

Measured, with the narrowing suppressed and the topology read forced to its fallback:

RealizedWidth { allowed: 8, cores: 8, restricted: false, workers: 4, ... }

allowed == cores passes there. And what happens next is worse than a vacuous pass — the run fails at workers == cores naming #1780, a resolver defect, on a host where the resolver was correct and the premise was never established. Green tells you nothing; red points at the wrong file. On a cpuset that already holds one CPU per core it passes outright.

This doesn't change your conclusion — the platform skip is still needed, for the reason you give. It changes what the skip is protecting: not a guard that would fail honestly off Linux, but one that would have been silently trivial there, the same way it can be silently trivial on Linux today.

#2084 has the child report restricted= and the parent require it (fail under NXRT_REQUIRE_PLACEMENT_TESTS=1, skip with a stated cause otherwise), keeps allowed == cores with a corrected comment — it still catches a leader set that isn't one-per-core on a host that answered — and replaces the two inline cfg!(target_os = "linux") spellings with one PROCESS_AFFINITY_MASKING_SUPPORTED constant that is asserted against the implementation that actually compiled.

The general rule I'd take from both PRs: a premise must be reported by whoever established it, never inferred from a consequence. Any inference asserts that no failure mode of the premise reproduces that consequence — and here one reproduced it exactly.