[Build] Fix windows build for sm90 or later - #29776
Conversation
25d1290 to
fc64ae3
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a Windows/MSVC CUDA build failure when targeting SM90+ (Hopper) by disabling compilation of the native Hopper TMA/WGMMA kernels (whose NVCC-generated host stubs can trigger MSVC C2719 due to 128-byte over-aligned by-value parameters). It also improves runtime behavior and documentation by providing clear early error messages when a model requests SM90-native prepacked weights on builds where those kernels are intentionally unavailable.
Changes:
- Disable
COMPILE_HOPPER_TMA_GEMMSandCOMPILE_HOPPER_TMA_GROUPED_GEMMSon MSVC builds for SM90+ in the CUDA provider and CUDA plugin CMake targets. - Add explicit early-fail checks/messages for
weight_prepacked=2(SM90-native layout) when the Hopper TMA kernel is not compiled in. - Update documentation to note that SM90-native fpA_intB kernels are not compiled on Windows/MSVC and describe the fallback behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| onnxruntime/test/util/default_providers.cc | Fixes preprocessor control flow/return paths for CUDA provider helpers (consistent nullptr returns when CUDA is unavailable). |
| onnxruntime/contrib_ops/cuda/quantization/matmul_nbits.h | Adds early runtime error for SM90-native prepacked weights when Hopper TMA kernels aren’t compiled. |
| onnxruntime/contrib_ops/cuda/llm/fpA_intB_gemm/launchers/fpA_intB_launcher_sm90.inl | Improves the stub error message on MSVC builds to explain native SM90 kernel unavailability and the supported workaround. |
| docs/contrib_ops/cuda/matmul_nbits.md | Documents Windows/MSVC limitation for SM90-native kernel compilation and expected fallback/layout guidance. |
| cmake/onnxruntime_providers_cuda.cmake | Avoids defining Hopper TMA compile macros on MSVC to prevent CUDA 13 host-stub/MSVC alignment build errors. |
| cmake/onnxruntime_providers_cuda_plugin.cmake | Mirrors the provider logic for the CUDA plugin build: do not define Hopper TMA compile macros on MSVC. |
Review summaryReviewed the full diff (6 files) plus the surrounding fpA_intB dispatch path and CI configuration. The fix is correct, well-scoped, and safe to merge. Notes below for the record and for anyone verifying it. What it does
Correctness — verified
CI interpretation
Notably, "Windows CUDA Plugin EP Build" (CUDA 13 + MSVC, archs How to verify it worksBecause the bug is a host compile-time error, the primary check is "does it compile" — no Hopper GPU required. The change is a no-op on non-MSVC (macro still defined for GCC/Clang), so Linux cannot regress by construction.
Minor nits (non-blocking)
|
|
@tianleiwu, |
…abilize test match prefix (#29797) ### Description `MatMulNBits.Fp16_Int4_PrepackedSm90BlockSize32Rejected` was failing in CUDA Plugin Linux CI because SM90 prepacked validation emitted environment-dependent messages before checking runner capability. This change makes rejection precedence portable across runner/build-machine combinations and stabilizes message matching via a shared prefix. - **Runtime-first SM90 gating** - For `weight_prepacked=2` (SM90 layout), validation now checks `sm_ == 90` **before** compile-time native-SM90 availability checks. - **Stable rejection prefix contract** - All SM90-prepacked rejection paths now use the exact prefix: - `weight_prepacked=2 (SM90 layout)` - **Build-support error clarity** - If `COMPILE_HOPPER_TMA_GEMMS` is unavailable, the post-runtime-check error explicitly reports missing native SM90 Hopper support. - **Block-size rule preserved** - SM90 prepacked still enforces `block_size` ∈ `{64, 128}` after runtime/build checks. - **Test portability update** - `Fp16_Int4_PrepackedSm90BlockSize32Rejected` continues asserting only the stable shared prefix so it passes on: - non-SM90 runners, - SM90 runners without native TMA support in the build, - SM90 runners with native TMA support (block-size rejection path). ```cpp if (weight_prepacked_ == kMatMulNBitsWeightPrepackedSm90) { ORT_ENFORCE(sm_ == 90, "weight_prepacked=2 (SM90 layout) ..."); #if !defined(COMPILE_HOPPER_TMA_GEMMS) ORT_THROW("weight_prepacked=2 (SM90 layout) is not supported by this ONNX Runtime build ..."); #endif ORT_ENFORCE(block_size_ == 64 || block_size_ == 128, "weight_prepacked=2 (SM90 layout) supports block_size 64 or 128 only, but got ", block_size_); } ``` ### Motivation and Context The failing CI test exposed that rejection order for SM90-prepacked weights depended on build configuration rather than first honoring runtime device capability, which is incorrect for SM90 layout execution on non-SM90 runners. This update aligns validation precedence with execution constraints and makes test matching architecture-agnostic while preserving precise downstream rejection semantics. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Review of the latest commit (
|
| Device | Build | First guard hit | Message prefix | Match |
|---|---|---|---|---|
| non-SM90 | any (MSVC or not) | sm_ == 90 enforce |
weight_prepacked=2 (SM90 layout) requires… |
✅ |
| SM90 | no native TMA (MSVC) | #if !defined throw |
weight_prepacked=2 (SM90 layout) is not supported… |
✅ |
| SM90 | native TMA (Linux) | block_size enforce |
weight_prepacked=2 (SM90 layout) supports block_size… |
✅ |
The reorder is behavior-preserving in the sense that matters (the model is still rejected in every case); it just reports device-incapability before build-support, which is more accurate and makes the test comment's three-way scenario description literally true. Removing "native" alone would fix the substring match; the reorder additionally improves message quality on non-Hopper machines. No new C4702 risk (the throw-then-enforce reachability is unchanged, and /wd4702 is disabled globally on MSVC anyway).
CI
CI has been re-triggered on the new head (9ee050b) and is mostly in progress with no failures yet; the previously-red Windows CUDA Plugin EP Build/Test and Windows GPU CUDA jobs are re-running. Worth a final glance once they settle, but the mechanism above explains and resolves the earlier failure.
Verdict
👍 The core Windows-build fix from commit 1 stands, and this commit correctly closes the test-portability gap it introduced. No outstanding concerns from me. (Nits from my first comment — the matmul_nbits.md anchor link and the out-of-scope default_providers.cc cleanup — remain minor and non-blocking.)
…tion (#29806) ## Summary Adds GPU-free unit coverage for the `weight_prepacked=2` (SM90/Hopper) rejection logic that #29776 / 9ee050b introduced in `MatMulNBits<T>`'s constructor, plus the minimal, behavior-preserving refactor needed to make that logic testable without a GPU. Stacked on `tlwu/fix_windows_build` (not `main`), since this tests code that only exists on that branch. ## Coverage gap `MatMulNBits<T>`'s constructor validates a `weight_prepacked=2` request inline with three checks, in order: 1. `ORT_ENFORCE(sm_ == 90, ...)` — device must be Hopper. 2. `#if !defined(COMPILE_HOPPER_TMA_GEMMS)` `ORT_THROW(...)` — the native SM90 kernel must actually be compiled into this build (it isn't on Windows/MSVC, which is exactly what #29776 changed). 3. `ORT_ENFORCE(block_size_ == 64 || block_size_ == 128, ...)` — block_size must be Hopper-tile-aligned. `sm_` is read from real device properties (`GetDeviceProp()`), and check 1 runs *before* check 2. So the "native SM90 kernel not compiled in this build" throw is only reachable on an actual Hopper GPU that was built without the native kernel (e.g. real Hopper hardware + MSVC). Every existing prepacked test (`Fp16_Int4_PrepackedSm90BlockSize32Rejected`, etc.) uses `OpTester` + `GTEST_SKIP()` when no CUDA device is present, so that branch has **no coverage at all** in CI today. ## Minimal refactor Extracted the validation into two pure, host-only functions that take `sm`/`block_size` as plain parameters instead of reading real hardware: - New header `onnxruntime/contrib_ops/cuda/quantization/matmul_nbits_sm90_validation.h` (declarations only, guarded by `#if USE_FPA_INTB_GEMM`): - `bool IsNativeSm90FpAIntBGemmCompiled();` - `void ValidateSm90PrepackedWeightSupport(int sm, int64_t block_size);` - Both are **defined non-inline** in `onnxruntime/contrib_ops/cuda/quantization/matmul_nbits.cc` (not in the header), so there is a single definition living inside the CUDA EP translation unit that actually observes `COMPILE_HOPPER_TMA_GEMMS` — avoiding an ODR/macro-skew hazard if a test TU saw a different value of that macro than the real target does. - `matmul_nbits.h`'s constructor now does: ```cpp if (weight_prepacked_ == kMatMulNBitsWeightPrepackedSm90) { ValidateSm90PrepackedWeightSupport(sm_, block_size_); } ``` replacing the inlined checks. **All three message strings are byte-identical** to before; only the identifiers changed from `sm_`/`block_size_` (members) to `sm`/`block_size` (parameters), which only affects the auto-stringified `ORT_ENFORCE` condition text, not the custom messages tests/users actually see. Net runtime behavior is unchanged. ## Test Added `onnxruntime/test/contrib_ops/cuda_kernels/matmul_nbits_sm90_validation_test.cc`, which calls `ValidateSm90PrepackedWeightSupport`/`IsNativeSm90FpAIntBGemmCompiled` directly with synthetic `(sm, block_size)` values — **no CUDA device required**: - `ValidateSm90PrepackedWeightSupport(80, 64)` throws containing `"weight_prepacked=2 (SM90 layout) requires a compute capability 9.0"` (build-independent). - If `IsNativeSm90FpAIntBGemmCompiled()`: `ValidateSm90PrepackedWeightSupport(90, 64)` does **not** throw; `ValidateSm90PrepackedWeightSupport(90, 32)` throws containing `"supports block_size 64 or 128 only"`. - Else (e.g. Windows/MSVC, or any non-Hopper `CMAKE_CUDA_ARCHITECTURES` build such as the sm86 used by `windows_cuda.yml`/`linux_cuda_ci.yml`): `ValidateSm90PrepackedWeightSupport(90, 64)` throws containing `"is not supported by this ONNX Runtime build"`. ### A deliberate deviation from a literal "add it to matmul_4bits_test.cc" — please sanity-check this I initially planned to add this `TEST()` directly inside `test/contrib_ops/matmul_4bits_test.cc` and call the new function straight from there. That does not link: `onnxruntime_providers_cuda` (and `_plugin`) is built as a **runtime-loaded shared module** (`onnxruntime_add_shared_library_module`), and on Windows its exports are restricted to a 3-symbol allowlist in `core/providers/cuda/symbols.def` (`GetProvider`, `CreateEpFactories`, `ReleaseEpFactory`). `matmul_nbits.cc`'s free functions are never exported from that DLL, and `onnxruntime_provider_test`/`onnxruntime_test_all` never link the CUDA provider module at compile time (`AddTest()`'s `DEPENDS` only does `add_dependencies()`, i.e. build-order, not `target_link_libraries()`). The established ORT mechanism for unit-testing CUDA EP internals directly is `test/contrib_ops/cuda_kernels/*.cc` (see sibling `fpA_intB_gemm_kernel_test.cc`, `softmax_topk_kernel_test.cc`): when `onnxruntime_ENABLE_CUDA_EP_INTERNAL_TESTS` is on, these sources are compiled into a separate `onnxruntime_providers_cuda_ut` shared module **together with** `$<TARGET_OBJECTS:onnxruntime_providers_cuda_obj>` (the CUDA EP's own object files) — same translation/link domain, so direct calls resolve, and the same `COMPILE_HOPPER_TMA_GEMMS` compile definition applies to both (via `config_cuda_provider_shared_module()`). This module's gtest cases run via the existing `TEST(CUDA_EP_Unittest, All)` nested-gtest bridge in `test/providers/cuda/cuda_provider_test.cc`. I confirmed both `windows_cuda.yml` and `linux_cuda_ci.yml` already set `onnxruntime_ENABLE_CUDA_EP_INTERNAL_TESTS=ON` and don't set `onnxruntime_BUILD_CUDA_EP_AS_PLUGIN`, so the new file is picked up automatically by the existing `file(GLOB ... contrib_ops/cuda_kernels/*.cc)` in `cmake/onnxruntime_unittests.cmake` — no CMake changes needed for those two legs. (Both legs build at `CMAKE_CUDA_ARCHITECTURES=86`, so `COMPILE_HOPPER_TMA_GEMMS` is undefined in both regardless of MSVC — they'll exercise the "not compiled" branch of the new test, which is exactly the previously-uncovered branch.) I added a short breadcrumb comment in `matmul_4bits_test.cc` next to `Fp16_Int4_PrepackedSm90BlockSize32Rejected` pointing at the new file, so this redirection is discoverable from where the original tests live. (Note: the hand-curated `onnxruntime_test_providers_cuda_plugin_internal_test_src` list used only when `onnxruntime_BUILD_CUDA_EP_AS_PLUGIN` is set was intentionally left untouched, to keep this change minimal — neither target CI leg uses that mode.) ## Verification - Did **not** attempt a full CUDA build (needs the CUDA toolkit + hours; out of scope here). - Wrote a standalone throwaway program (not checked in) reproducing the extracted function bodies with minimal `ORT_ENFORCE`/`ORT_THROW` stand-ins, and compiled + ran it twice with MSVC (`cl.exe`) — once with `/DCOMPILE_HOPPER_TMA_GEMMS`, once without — confirming both preprocessor branches compile cleanly and all three message strings match exactly what's specified above. - Ran `lintrunner -a` on all changed files: no issues. - Re-read the diff for ODR correctness (both new functions are non-inline with a single definition in `matmul_nbits.cc`), guard placement (`#if USE_FPA_INTB_GEMM` around declarations/definitions, `#if !defined(COMPILE_HOPPER_TMA_GEMMS)` around the build-support throw), and message byte-identity. - Relying on ONNX Runtime CI (`windows_cuda.yml` / `linux_cuda_ci.yml`) for full build/link/run validation of the new test in its actual target environment. Do not merge — opening for review/discussion. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Gopalakrishnan Nallasamy <gopalakrishnan.nallasamy@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Build error in Windows MSVC for sm90 for later:
The fix is to exclude TMA from Windows build.