Preserve CUDA internal-test builds without the Windows CMake cycle by xadupre · Pull Request #32805 · microsoft/onnxruntime

Xavier Dupré (xadupre) · 2026-09-25T15:28:43Z

Break the Windows CUDA internal-test dependency cycle while preserving --target onnxruntime_provider_test: an aggregate target builds the executable and its dynamically loaded test module. Binary and CTest names remain unchanged.

Correct option ordering so fresh configurations enable the requested internal tests.
Fix three MSVC warnings exposed by compiling those tests (workspace size, float literal, profiling threshold overflow).
Before the full Windows build, verify that the provider-test target alone produces both artifacts from fresh outputs.
Explicitly run CUDA_EP_Unittest.All on the GPU runner and reject missing or skipped tests.

Local checks: CMake generation, PowerShell fixtures and lint; no ORT compilation. Windows CI has passed generation and built the host executable; module linking and execution remain to be validated.

Keep the module-to-host link dependency on Windows and remove the reverse build-order edge from the provider test executable. Document the module target as the targeted Windows internal-test build entry point. Fixes #32804 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Guard the shared dependency-list append at its source instead of filtering the provider-test copy later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟢 Approval recommended

The focused dependency adjustment resolves the reported cycle without altering non-Windows or plugin configurations.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes the Windows CUDA internal-test CMake dependency cycle while preserving module linking and default-build behavior.

Changes:

Removes the Windows-only executable-to-module dependency.
Documents targeted Windows internal-test builds.

File	Description
`cmake/onnxruntime_unittests.cmake`	Breaks the cyclic target dependency on Windows.
`docs/contrib_ops/cuda/matmul_nbits.md`	Documents the required module build target.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Build the dynamically loaded module explicitly for targeted builds instead of making it a prerequisite of the test executable. Retain the Windows module-to-executable import-library link. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Separate runtime module prerequisites from binary build dependencies. In the Windows non-plugin internal-test configuration, keep onnxruntime_provider_test as the public aggregate target and link the module against a separate executable target with the original output filename. Preserve CTest names, environment, timeout, reporting and executable PCH settings. Other configurations retain their executable target and module build dependency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The Windows-specific executable/import-library linkage still requires native Windows CI validation.

Review effort: Balanced
Findings: None

Evaluate the internal-test option after its prerequisites. Reject a missing test module in the Windows build job and explicitly run the wrapper on the GPU runner, requiring a fresh completed-test XML result so absent or skipped tests cannot pass silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The Windows-specific executable/import-library dependency graph still requires native Windows CI validation.

Review effort: Balanced
Findings: None

Separate configure from the full build and first build only the public provider-test target. Require fresh executable and CUDA test module outputs so the full build cannot mask a missing aggregate dependency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The platform-specific target graph and symbol-resolution changes require the pending native Windows CUDA CI validation.

Review effort: Balanced
Findings: None

Preserve size_t workspace sizes, use a float comparison literal, and widen the tactic-pruning threshold multiplication so the FLT_MAX no-best sentinel cannot overflow during constant folding. Keep warnings-as-errors enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The Windows-specific import-library linkage and runtime module loading still require successful end-to-end CI validation.

Review effort: Balanced
Findings: None

Ti-Tai Wang (titaiwangms) · 2026-09-25T22:12:45Z

Review of head e4d41c85 — one blocking finding.

Critical — Windows CUDA internal-test module fails to link (cmake/onnxruntime_unittests.cmake:1491–1502). The module links against the test executable and therefore needs its Windows import library. At this head, CI successfully links onnxruntime_provider_test.exe, then fails with LNK1181: cannot open input file RelWithDebInfo\onnxruntime_provider_test.lib while linking onnxruntime_providers_cuda_ut. ENABLE_EXPORTS alone does not ensure the executable exports symbols or produces an import library. Please export the symbols required by the module (for example through explicit exports or a definition file), or change the module/executable dependency, and confirm the fresh targeted build produces both artifacts. The Windows GPU test job is currently skipped because the build failed. CI: https://github.com/microsoft/onnxruntime/actions/runs/36165378901/job/108171731884

Non-blocking question: The new XML check proves that the outer CUDA_EP_Unittest.All wrapper completed, but does not prove that any internal module tests actually ran rather than all being skipped. If inner-test coverage is the intended guarantee, consider asserting a nonzero inner test count or otherwise checking the inner results.

No other confirmed defects from the full review. This was a static review with PR-head CI-log verification; no local Windows build or GPU run was performed.

Checkpoint the current incomplete fix. Automatic exports do not cover linked static-library symbols, and the test-count check runs before Google Test selects tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Compile internal-test objects before the host link and derive targeted exports from module references and static host libraries. Keep tests in the DLL, add export-generation regressions and CI artifact checks, and verify successful internal tests after execution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Emit a generated source containing /INCLUDE linker directives for the selected exports. This ensures MSVC extracts the required static-library members even when the host itself does not reference them, without exporting or forcing inclusion of unrelated symbols. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Match CMake's DEF writer by emitting decorated symbol names directly. Add native linker library diagnostics and dump the generated DEF and EXP symbol table on fixture failure so export-name mismatches can be distinguished from archive resolution failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Keep tests in the module and call provider Node/Tensor implementations through statically linked opaque-pointer test adapters. Initialize the module's C++ API consistently, import host test utilities instead of duplicating ort_env-dependent objects, and link Windows ONNX protobuf definitions directly. Collect exports from actual host link inputs, including imported and object libraries. Extend the MSVC fixture to exercise class/struct adapters, separate API initialization modes, DLL loading, and incremental exports. Validation: rebuilt the actual Linux CUDA test module and passed all 126 selected tests with no skips. Generator regressions and Windows COFF linking passed locally; full native Windows validation remains for CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The cross-DLL symbol-export path is high risk and final GPU execution validation is still pending.

Review effort: Balanced
Findings: None

Ti-Tai Wang (titaiwangms) · 2026-09-28T21:05:46Z

Review of updated head 263ce0e5: the previous Windows link blocker is resolved. The Windows CUDA targeted build now produces onnxruntime_provider_test.exe, its import library, and onnxruntime_providers_cuda_ut.dll; the GPU job ran 295 internal tests (291 passed, 4 skipped). Linux CUDA also ran the internal-test wrapper successfully.

Minor failure-path finding — guard failed temporary-file creation in the CUDA module tests. The revised cmake/onnxruntime_unittests.cmake:1042-1047 stops linking onnxruntime_test_utils into the module. On Windows, CreateTestFile consequently runs from the host executable and reports its fatal GTest assertions to the host copy of GTest, not the module copy. If _wmktemp_s or _wfopen_s fails in onnxruntime/test/util/file_util.cc:53-69, it returns before assigning the output pointer. The module-side ASSERT_NO_FATAL_FAILURE(CreateTestFile(model_file, model_path)) at onnxruntime/test/providers/cuda/test_cases/cuda_external_data_loader_test.cc:326 cannot observe that other GTest instance’s failure, so the test reaches fwrite(..., model_file) at line 330 with model_file == nullptr. The other CreateTestFile call at line 49 has the same unchecked-null consequence. Please add an explicit module-side ASSERT_NE(file, nullptr) after both calls (before any fwrite), or otherwise keep the assertions and error handling in the same GTest instance. This is a statically identified error path, not a failure observed in the passing CI run.

I would hold approval until that narrow failure path is addressed; the full-team review found no other confirmed blocking defects. I did not run a local Windows build or GPU test.

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

Account for the sparse initializer validation added on main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Xavier Dupré (xadupre) · 2026-09-29T10:05:49Z

Copilot resolve the merge conflicts in this pull request

…rnal-tests-cycle # Conflicts: # .github/workflows/android.yml Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>

Copilot · 2026-09-29T10:08:20Z

Copilot resolve the merge conflicts in this pull request

Merged main and resolved the Android workflow threshold conflict, retaining the higher PR threshold. Merge commit: 0df744e4.

Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>

Replace the stale routing record reference in the packed INT GEMV path and preserve expert selection collection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

A supported mixed WebGPU/CUDA Windows configuration fails CMake, and packed INT GEMV tracking lacks regression coverage.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Use the valid provider test target for WebGPU definitions on Windows · New
Add packed INT GEMV MoE expert counting regression test · New

Use the executable target for WebGPU definitions and cover packed INT GEMV expert counting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

The new packed INT GEMV regression uses unsupported dimensions and therefore fails instead of exercising the intended path.

Review effort: Balanced
Findings: 1

Open (1)

Use dimensions that exercise the packed INT GEMV path · New

Resolved since last review (2)

Use the valid provider test target for WebGPU definitions on Windows
Add packed INT GEMV MoE expert counting regression test

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

Copilot

Copilot review overview

🔵 Needs a closer look

The platform-specific export and dynamic-module linking path still requires completed Windows CI validation on the current head.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Use dimensions that exercise the packed INT GEMV path

Copilot

Copilot review overview

🔵 Needs a closer look

The Windows executable-import-library/module link path and CUDA execution still require full CI validation.

Review effort: Balanced
Findings: None

Copilot AI balanced review requested due to automatic review settings September 25, 2026 15:28

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 25, 2026 15:29 View session

Avoid adding the Windows CUDA test back dependency

ae177b5

Guard the shared dependency-list append at its source instead of filtering the provider-test copy later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Xavier Dupré (xadupre) and others added 2 commits September 25, 2026 15:35

Xavier Dupré (xadupre) changed the title ~~Fix Windows CUDA internal-test CMake dependency cycle~~ Preserve CUDA internal-test builds without the Windows CMake cycle Sep 25, 2026

Xavier Dupré (xadupre) requested a balanced review from Copilot September 25, 2026 15:47

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 25, 2026 15:48 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Xavier Dupré (xadupre) requested a balanced review from Copilot September 25, 2026 15:58

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 25, 2026 15:59 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Xavier Dupré (xadupre) requested a balanced review from Copilot September 25, 2026 16:13

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 25, 2026 16:14 View session

Xavier Dupré (xadupre) mentioned this pull request Sep 25, 2026

[Build] Windows CUDA internal tests fail CMake generation due to a circular target dependency #32804

Open

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Xavier Dupré (xadupre) requested a review from Ti-Tai Wang (titaiwangms) September 25, 2026 16:36

Xavier Dupré (xadupre) requested a balanced review from Copilot September 25, 2026 17:11

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 25, 2026 17:11 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Xavier Dupré (xadupre) and others added 2 commits September 28, 2026 10:44

Xavier Dupré (xadupre) requested a balanced review from Copilot September 28, 2026 12:23

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 28, 2026 12:23 View session

Xavier Dupré (xadupre) and others added 3 commits September 28, 2026 13:33

Xavier Dupré (xadupre) requested a balanced review from Copilot September 28, 2026 16:19

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 28, 2026 16:19 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Xavier Dupré (xadupre) and others added 2 commits September 29, 2026 09:38

Guard CUDA test temporary files

d57c3d4

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

Raise Android minimal binary size threshold

7a37727

Account for the sparse initializer validation added on main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot started work on behalf of Xavier Dupré (xadupre) September 29, 2026 10:06 View session

Merge remote-tracking branch 'origin/main' into fix/windows-cuda-inte…

0df744e

…rnal-tests-cycle # Conflicts: # .github/workflows/android.yml Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>

Limit Windows ARM64 workflow token permissions

3dfc0ce

Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>

Copilot finished work on behalf of Xavier Dupré (xadupre) September 29, 2026 10:10

Fix QMoE routing snapshot build

a27bfdd

Replace the stale routing record reference in the packed INT GEMV path and preserve expert selection collection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Xavier Dupré (xadupre) requested a balanced review from Copilot September 29, 2026 11:54

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 29, 2026 11:55 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Comment thread cmake/onnxruntime_unittests.cmake

Comment thread onnxruntime/contrib_ops/cuda/moe/moe_quantization.cc

Address provider target review feedback

d978b23

Use the executable target for WebGPU definitions and cover packed INT GEMV expert counting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Xavier Dupré (xadupre) requested a balanced review from Copilot September 29, 2026 12:18

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 29, 2026 12:19 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Comment thread onnxruntime/test/framework/moe_expert_counting_test.cc Outdated

Fix packed INT GEMV counting test shape

347b088

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

Xavier Dupré (xadupre) requested a balanced review from Copilot September 29, 2026 13:03

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 29, 2026 13:04 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Xavier Dupré (xadupre) requested a balanced review from Copilot September 29, 2026 13:21

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 29, 2026 13:22 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Conversation

Xavier Dupré (xadupre) commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Ti-Tai Wang (titaiwangms) commented Sep 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Ti-Tai Wang (titaiwangms) commented Sep 28, 2026

Uh oh!

Xavier Dupré (xadupre) commented Sep 29, 2026

Uh oh!

Copilot AI commented Sep 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Xavier Dupré (xadupre) commented Sep 25, 2026 •

edited

Loading