Follow standard CMake semantics for CUDA architecture suffixes#29601
Follow standard CMake semantics for CUDA architecture suffixes#29601Sammy-Dabbas wants to merge 1 commit 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., |
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.