Skip to content

test(ep-cuda): list the cuFFT DFT sync, and check the allowlist's premise (fixes CUDA compile lane) - #2099

Merged
justinchuby merged 1 commit into
mainfrom
squad/gaff-cuda-capture-contract
Aug 25, 2026
Merged

justinchuby merged 1 commit into
mainfrom
squad/gaff-cuda-capture-contract

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

CUDA compile (Linux x86_64) has been red on main since #2080, and every branch cut from it inherits the failure (seen on #2084: job 97720816964). The set difference is exactly one entry:

left  contains "dft.rs::run"
right does not

The sync is legitimate; the omission was the list

#2080's cuFFT DFT kernel drains the compute stream before its metadata upload:

// The metadata prefix belongs to step-scoped workspace and may be
// reused by the next dispatch. Drain the non-blocking compute stream
// before the synchronous default-stream upload so it cannot overwrite
// metadata still consumed by a prior DFT. This host barrier is one
// reason capture remains explicitly unsupported.

and DftKernel::capture_support returns CaptureSupport::unsupported(…). So it qualifies under the contract's own rule — capture-unsupported paths are listed rather than guarded. Added with that justification.

The rest of the PR: the allowlist's premise was never checked

Listing the entry is the whole fix for the red lane. But the allowlist carried its justification in a comment —

// Every entry is a path whose capture_support is explicitly Unsupported, or a dynamically-admitted kernel's fallback path that capture_support rejects.

— and nothing checked it. That makes the list an unconditional escape hatch: a kernel advertising CaptureSupport::Supported with an unguarded .synchronize() is silenced by one line, graph capture breaks, and the suite goes green. That is a worse outcome than the failure the contract exists to catch, because the contract is the only thing looking.

every_allowlisted_file_can_decline_capture turns the sentence into an assertion. Its limit is stated in its own doc comment: it resolves capture_support per file, not per kernel, because function_blocks is a flat scan with no impl awareness. It is a lower bound and says so.

Falsified in both directions

The contract test is a pure source scan, so it reproduces without --features cuda and without a GPU — that is how CI's failure was reproduced here byte-identically before any change.

mutation result
drop "dft.rs::run" from the list contract test FAILS with CI's exact left/right sets — the entry is load-bearing
make dft.rs's capture_support return Supported, entry still listed every_allowlisted_file_can_decline_capture FAILS — not vacuous

The second row is the one worth reading twice: under that mutation the pre-existing contract test still reports ok. That is the silent escape hatch, demonstrated rather than asserted.

Validation

cargo test --locked -p onnx-runtime-ep-cuda --features cuda --test capture_sync_contract   2 passed; 0 failed
cargo clippy --locked -p onnx-runtime-ep-cuda --features cuda --tests -- -D warnings       clean
cargo fmt --all -- --check                                                                 clean

The first is CI's exact command from the failing step. Test-only change; no production source is touched, so nothing here alters kernel behaviour.

Class

Catalogue #1817: a claim carried in prose where a check was available. The allowlist asserted its own entries were reviewed and had no way to be wrong about it.

…mise

#2080 added an unconditional `stream().synchronize()` to `dft.rs::run`
without adding it to the capture-sync contract's allowlist, so
`CUDA compile (Linux x86_64)` has been red on main and on every branch
cut from it since. The set difference is exactly one entry.

The sync is legitimate. `DftKernel::capture_support` returns
`CaptureSupport::unsupported`, and the barrier's own comment names it as
one of the reasons: the step-scoped metadata prefix may be reused by the
next dispatch, so the compute stream is drained before the synchronous
default-stream upload. Listed with that justification.

Listing it is the whole fix for the red lane. The second commit half is
about what listing means. The allowlist carried the sentence "Every
entry is a path whose capture_support is explicitly Unsupported" in a
comment, where nothing checked it — so the list was also an
unconditional escape hatch: one line silences the contract for a kernel
that advertises `CaptureSupport::Supported`, and graph capture then
breaks with the suite green. Silently, which is worse than the failure
the contract exists to catch.

`every_allowlisted_file_can_decline_capture` turns that sentence into an
assertion. Its limit is stated in its own doc comment: it resolves
`capture_support` per file, not per kernel, because the source scan is
flat and has no `impl` awareness, so it is a lower bound.

Falsified both directions, without a GPU — the contract test is a source
scan and runs without `--features cuda`, which is how the CI failure was
reproduced here byte-identically:

  - drop `dft.rs::run` from the list: the contract test FAILS with CI's
    exact left/right sets, so the entry is load-bearing;
  - make `dft.rs`'s `capture_support` return `Supported` while leaving
    the entry listed: `every_allowlisted_file_can_decline_capture`
    FAILS, so it is not vacuous.

cargo test --locked -p onnx-runtime-ep-cuda --test capture_sync_contract:
2 passed, 0 failed. fmt clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby

Copy link
Copy Markdown
Owner Author

Expect Rust coverage (macOS arm64) to be red on this PR, and it is not caused by anything here — it is #2083's STFT assertion on the radix-2 counter, which Apple targets never reach for n >= 4. It is red on main and on every branch cut from it; #2097 fixes it. This PR touches only crates/onnx-runtime-ep-cuda/tests/capture_sync_contract.rs.

The lane this PR fixes is CUDA compile (Linux x86_64).

— Gaff

@codecov

codecov Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.60%. Comparing base (46478fc) to head (19c1271).
⚠️ Report is 35 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2099      +/-   ##
==========================================
+ Coverage   80.57%   80.60%   +0.02%     
==========================================
  Files         412      428      +16     
  Lines      193684   209225   +15541     
  Branches   193684   209225   +15541     
==========================================
+ Hits       156064   168642   +12578     
- Misses      32136    34889    +2753     
- Partials     5484     5694     +210     
Flag Coverage Δ
cli-ort-linux 72.51% <ø> (?)
cli-ort-windows 72.10% <ø> (?)
mlas 85.80% <ø> (?)
offline 80.72% <ø> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 76 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant