Repository navigation
fix(cuda): let the VMM reservation ladder descend under managed no-spill - #1608
Merged
Merged
Conversation
The arena reservation ladder exists so "a platform with a tighter address
space still gets a ledgered arena instead of silently dropping to the
unaccounted cuMemAlloc fallback". Under managed no-spill it never descended:
the first rung's failure was turned into a hard error and propagated out of
the loop by `?`, so the smaller rungs were never attempted.
That is the configuration where the ladder matters most, because there the
arena is *required* rather than best-effort.
Measured on an RTX 4060 Laptop (8 GiB). `RESERVATION_FLOOR_BYTES` floors the
first rung at 1 TiB for any small card, the driver refuses it, and native
decode failed to load at all:
managed no-spill CUDA initialization failed before model allocation ...
cannot reserve 1099511627776 bytes of device memory:
cuMemAddressReserve failed: CUDA_ERROR_OUT_OF_MEMORY
This only reproduced through paths that enable managed no-spill, so
`profile_native --steady` failed while a plain non-steady run of the same
binary and model worked. Since `--steady` is what the native-vs-ORT
comparison uses, the native arm errored out and only the ORT arm reported a
number — which reads as "native is slow" rather than "native did not run".
Fix: resolve every rung in fallback mode so a failure records the error and
lets the loop try the next, smaller rung, and defer the no-spill verdict
until the ladder is exhausted. The failure message and error kind are
unchanged, being re-raised through the same formatter.
After the fix the ladder descends one rung to 512 GiB and native loads:
cuda_ep: device allocations go through a VMM arena over 549755813888
bytes of reserved address space
and the comparison it was blocking now runs on this box (qwen2.5-0.5B,
128 tokens, 2 warmups, 3 runs, quiet box): native 379.37 tok/s vs ORT-CUDA
169.76 tok/s, native 2.23x faster.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
justinchuby
added a commit
that referenced
this pull request
Aug 20, 2026
Brings in #1604 (CI repair for stable 1.98.0) and #1608 (VMM reservation ladder under managed no-spill). One content conflict: crates/onnx-runtime-ep-cuda/src/provider.rs. Resolved to this branch's side, because the defect #1608 fixes cannot occur on this branch. #1608 fixed a loop that resolved every ladder rung through `resolve_vmm_initialization(auto_dynamic_lending, ..)?`, so under managed no-spill the *first* rung's failure returned Err and the smaller rungs were never attempted. Its fix resolves each rung in fallback mode and defers the fatal verdict to after the loop. This branch removed the two-mechanism selection entirely: the arena is the only built-in mechanism, `resolve_vmm_initialization` does not exist here (0 occurrences), and the loop calls `build_arena` directly, recording `Err` into `last_error` and continuing. The ladder therefore always descends fully, in every configuration, and only exhausting it is fatal. #1608's intent is satisfied unconditionally rather than conditionally. The two sides differ in one deliberate way, which is not a merge loss: when the ladder is exhausted and no-spill was not requested, main warns and falls back to the eager allocator, whereas this branch fails the provider. That is the point of the stack -- there is no second mechanism to fall back to. The error path is `vmm_unavailable`, which states the support boundary and appends the requested managed limit when one was set, so the no-spill case is still reported as such. main made no other change to this file; its diff against the merge base is exactly the two hunks above. Audit, mechanical rather than by eye: - every file main changed outside the overlap set: blob hash compared against origin/main, all identical - the five files #1604 reformatted for 1.98.0: now byte-identical to origin/main, so this branch's earlier 1.97 formatting of the same files is superseded rather than mixed - Cargo.toml: every line main added is present in the merged result, including the central `chunks_exact_to_as_chunks` allow; this branch's three onnx-runtime-memory-api entries survive, and that crate already carries `[lints] workspace = true`, so the allow reaches it Not addressed here, on purpose: main pins no toolchain, which is the cause behind both #1604 symptoms. Tracked in #1600, not widened into this branch.
This was referenced Aug 20, 2026
Closed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1608 +/- ##
==========================================
- Coverage 80.94% 80.79% -0.16%
==========================================
Files 382 380 -2
Lines 178556 175073 -3483
Branches 178556 175073 -3483
==========================================
- Hits 144528 141445 -3083
+ Misses 29089 28731 -358
+ Partials 4939 4897 -42
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
justinchuby
added a commit
that referenced
this pull request
Aug 20, 2026
…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>
justinchuby
added a commit
that referenced
this pull request
Aug 20, 2026
…hosts cannot take (#1610) Fixes the `Rust coverage` failure in `kernels::matmul::tests::half_prefill_gebp_agrees_with_the_blocked_half_gemm_and_is_the_route`. ## What the failure was ``` matmul.rs:5434: assertion `left == right` failed: BFloat16 m=2: prefill did not take the fused widen-pack GEBP left: 0 right: 1 ``` **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 at `eb8ce595` (#1604) and red at `a429538f` (#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-cuda` at 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_tile` tries the native AVX-512 BF16 kernel *before* the GEBP and returns: ```rust if format == HalfFormat::Bf16 && x86_bf16::native_available() { x86_bf16::gemm(a, b, c, m, k, n); return; // never reaches the GEBP route } if half_prefill_gebp_selected(format, m, k, n) && half_prefill_gebp_enabled() { ... } ``` 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 in `half_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 a **`continue`**, so on a non-AVX2/FMA host it skipped the numeric comparison against `naive_matmul` as 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_tile` actually dispatches on**, rather than restating hardware conditions in the test: ```rust let expect_native_bf16 = format == HalfFormat::Bf16 && x86_bf16::native_available(); let expect_gebp = !expect_native_bf16 && half_prefill_gebp_selected(format, m, k, n) && half_prefill_gebp_enabled(); assert_eq!( (half_prefill_gebp_calls(), half_native_bf16_calls()), (u64::from(expect_gebp), u64::from(expect_native_bf16)), ...); ``` - A test-only `HALF_NATIVE_BF16_CALLS` 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 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. - **The numeric comparison against `naive_matmul` now runs unconditionally on every host.** - Because the expectation reuses the real predicates, it cannot drift from the dispatch order the way a transcribed condition would. ## 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** of `native_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")]` and `x86_bf16::native_available()` does not exist here. Cross-compiling is blocked in both directions: `x86_64-apple-darwin` because upstream publishes no `onnxruntime-osx-x86_64-1.28.0.tgz` (HTTP 404), and `x86_64-unknown-linux-gnu` because ort-sys bindgen fails on `'stdlib.h' file not found`. `onnx-genai-ort-sys` is a normal dependency via `onnx-runtime-ep-api`, so `--lib` does 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 have `avx512bf16`. 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 on `Rust coverage (Linux x86_64)` *and* `Rust 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 - Touches only `crates/onnx-runtime-ep-cpu/src/kernels/matmul.rs`. No overlap with #1579's file set; nothing in `crates/onnx-genai-engine/src/{decode,native_decode}/`. - Based on `a429538f`, which predates #1609, so this branch inherits `main`'s `CUDA compile (Linux x86_64)` red at `Clippy CUDA execution provider`. That failure is **not** from this change — #1609 fixes it. Refs #1600. --------- Co-authored-by: Copilot App <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.
The bug
The arena reservation ladder exists so "a platform with a tighter address space still gets a ledgered arena instead of silently dropping to the unaccounted
cuMemAllocfallback" (its own comment). Under managed no-spill it never descended.resolve_vmm_initializationturns a failure into a hardErrwhenmanaged_no_spillis set, and the loop propagates it with?:So the ladder was defeated in exactly the configuration that needs it most — the one where the arena is required rather than best-effort.
Measured impact (RTX 4060 Laptop, 8 GiB)
RESERVATION_FLOOR_BYTESfloors the first rung at 1 TiB for any small card (there is even a test asserting this for the 4060). The driver refuses it, and native decode failed to load at all:Why this was easy to misread as a performance problem. It only reproduces through paths that enable managed no-spill, so
profile_native --steadyfailed while a plain non-steady run of the same binary and model worked fine (207 tok/s, capture on).--steadyis what the native-vs-ORT comparison uses — so the native arm errored out, only the ORT arm printed a number, and the result reads as "native is slow" rather than "native did not run".The fix
Resolve every rung in fallback mode so a failure records the error and lets the loop try the next, smaller rung; defer the no-spill verdict until the ladder is exhausted. The failure message and error kind are unchanged — the exhausted case is re-raised through the same formatter.
Verified (measured, quiet box)
cuda_ep: device allocations go through a VMM arena over 549755813888 bytes of reserved address space--steadynative now runs: 379.37 tok/s,cuda_graph: enabled=true captures=5 replays=630 fallbacks=0.cargo fmt -p onnx-runtime-ep-cuda -- --checkexits 0.Pre-existing failures, not caused by this change
provider::GPU tests fail on this box with and without the change — sameCUDA_ERROR_ILLEGAL_ADDRESSsymptom, one test's context poisoning the next:main(control)Counts move run to run in both arms, so these are pre-existing and flaky, not a regression here. Not fixed in this PR — worth its own investigation.
Not claimed
Whether the same ladder failure occurs on large-VRAM devices (H200): a 143 GiB card asks for 2 TiB via the
16 x VRAMmultiple, which may or may not be refused. Untested — no such device here.