Follow standard CMake semantics for CUDA architecture suffixes#29601
Follow standard CMake semantics for CUDA architecture suffixes#29601Sammy-Dabbas wants to merge 2 commits into
Conversation
setup_cuda_architectures kept only the last -virtual entry and dropped PTX for suffix-less architectures, so 86-virtual;120-virtual embedded PTX for 12.0 only and bare 86 became 86-real without a virtual arch. Track real and virtual architectures in separate lists: a suffix-less arch enables both real and virtual (standard CMake), every -virtual entry is preserved, and the accelerated (a) and family-code (f) handling apply to both. CMAKE_CUDA_ARCHITECTURES_ORIG becomes the deduplicated union so PTX-enabled archs also get their SM-specific kernels.
There was a problem hiding this comment.
Pull request overview
This PR fixes ONNX Runtime’s CUDA architecture normalization in setup_cuda_architectures to follow standard CMake semantics, ensuring both SASS (-real) and PTX (-virtual) handling is correct and that multiple -virtual entries are preserved (fixing #29598).
Changes:
- Split normalization into separate “real” and “virtual” cleaned lists so multiple
-virtualentries are retained. - Make suffix-less architectures (e.g.,
86) enable both86-realand86-virtualas standard CMake does. - Normalize accelerated (
a) and family (f) handling across both real and virtual variants and expose the deduplicated union viaCMAKE_CUDA_ARCHITECTURES_ORIG.
| set(CMAKE_CUDA_ARCHITECTURES_LAST_VIRTUAL ${CUDA_ARCH}) | ||
| if(CUDA_ARCH MATCHES "^(([1-9])([0-9])+[af]?)-virtual$") | ||
| list(APPEND CMAKE_CUDA_ARCHITECTURES_VIRTUAL_CLEAN ${CMAKE_MATCH_1}) | ||
| elseif(CUDA_ARCH MATCHES "^(([1-9])([0-9])+)[af]?-real$") |
| endif() | ||
| endforeach() | ||
|
|
||
| set(CMAKE_CUDA_ARCHITECTURES ${CMAKE_CUDA_ARCHITECTURES_NORMALIZED}) |
|
Thanks for writing the PR. FYI I also discovered today that the logic of forcing all "accelerated" archs (90, 100, 101, 110, 120) to the
So for example, if you select At present this can be circumvented by adding at least one "non-accelerated" virtual architecture (e.g., |
The setup_cuda_architectures macro forced the accelerated `a` suffix onto virtual (PTX) architectures. Architecture-conditional PTX built as compute_XXa is not forward compatible, so a selected 120-virtual became 120a-virtual and its embedded PTX would not JIT onto newer GPUs. Drop the accel branch in the virtual normalization loop so virtual archs keep the user-specified variant. Real (SASS) architectures still use the `a` suffix because the ORT kernels need the accelerated features such as WGMMA and TMA, and SASS never JITs forward regardless, so this costs no forward compatibility. Fix the -real regex so the optional family or accel suffix sits inside the capture group. The previous pattern left [af] outside the group, so 100f-real captured only 100 and was rewritten to 100a-real, silently dropping the family code. It now mirrors the -virtual branch and yields 100f. Deduplicate the normalized architecture list. A suffix-less family code such as 100f is pushed onto both the real and virtual clean lists, and both loops emit a bare 100f, which produced a duplicate in the final output.
|
Thanks for catching this. The a-forcing on virtual architectures was introduced by this PR through the new virtual normalization loop, I have removed it, so a plain 120-virtual stays a 120-virtual and its compute_120 PTX remains forward compatible via JIT on newer GPUs. The a-forcing on real architectures is kept because the ORT kernels need the accelerated SASS features such as WGMMA and TMA, and SASS is architecture specific anyway, so this does not cost any forward compatibility that PTX would otherwise provide. The fix only stops auto adding the a suffix. An explicit Xa-virtual is preserved unchanged, so the accelerated PTX variant is still available when it's requested directly. |
Description
setup_cuda_architecturesincmake/external/cuda_configuration.cmakewas non-standard in two ways (#29598): only the last-virtualentry was kept, so86-virtual;120-virtualdropped PTX for 8.6; and a suffix-less arch like86became86-realonly, dropping its virtual arch.This tracks real and virtual architectures in separate lists. A suffix-less arch enables both (standard CMake:
86gives86-realand86-virtual),-real/-virtualentries are kept as given, and all-virtualentries are preserved. The acceleratedaand family-codefhandling apply to both, andCMAKE_CUDA_ARCHITECTURES_ORIGis the deduplicated union.Motivation and Context
Fixes #29598. Thanks @cschreib-ibex for the reference patch; this builds on it, fixing that its
-virtualregex only matched even digit counts (so100-virtual/120-virtualfatal-errored) and predated theffamily branch. Verified the list logic with acmake -Pharness; nvcc compilation should be confirmed by CI.