build: Switch from FindCUDA to FindCUDAToolkit (#2072) by MarkLeone · Pull Request #2073 · AcademySoftwareFoundation/OpenShadingLanguage

MarkLeone · 2026-02-11T18:38:49Z

FindCUDA is obsolete. FindCUDAToolkit was added in CMake 3.17. Current cmake_minimum_required is already 3.19.

…tion#2072) FindCUDA is obsolete. FindCUDAToolkit was added in CMake 3.17. Current cmake_minimum_required is already 3.19. Signed-off-by: Mark Leone <mleone@nvidia.com>

linux-foundation-easycla · 2026-02-11T18:38:56Z

The committers listed above are authorized under a signed CLA.

✅ login: MarkLeone / name: Mark Leone (30c135a)

lgritz · 2026-02-14T02:57:10Z

After kicking off a re-run of the failed CI test, it passes now, so I think that was unrelated to your PR.

Copilot

Pull request overview

Updates the project’s CUDA discovery logic to use CMake’s modern FindCUDAToolkit module (since FindCUDA is deprecated/obsolete), aligning with the repo’s existing CMake minimum version.

Changes:

Switch CUDA detection in src/cmake/externalpackages.cmake from CUDA (FindCUDA) to CUDAToolkit (FindCUDAToolkit) and add legacy-variable bridging.
Update the CUDA example testsuite to use find_package(CUDAToolkit) and replace the removed FindCUDA cuda_compile_ptx functionality with a local CMake function.
Prefer imported CUDA targets (e.g., CUDA::cudart(_static), CUDA::nvrtc) when available.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
testsuite/example-cuda/CMakeLists.txt	Switches example to `CUDAToolkit`, adds a custom PTX compile function, and updates CUDA variables/targets used for linking.
src/cmake/externalpackages.cmake	Replaces FindCUDA with FindCUDAToolkit and adds a compatibility bridge for legacy `CUDA_*` variables/behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-28T03:03:00Z

 checked_find_package(OpenImageIO 2.4 REQUIRED)
 checked_find_package(OptiX REQUIRED)

+set(CUDA_TOOLKIT_ROOT_DIR ${CUDAToolkit_ROOT_DIR})


CUDAToolkit_ROOT_DIR doesn’t appear to be set anywhere in this repo and is not one of the variables exposed/used elsewhere for FindCUDAToolkit. As written, CUDA_TOOLKIT_ROOT_DIR will likely be empty, which then defeats the find_library(... HINTS ${CUDA_TOOLKIT_ROOT_DIR} ...) calls below. Consider deriving the toolkit root from CUDAToolkit_TARGET_DIR / CUDAToolkit_BIN_DIR (parent dir) / CUDAToolkit_LIBRARY_ROOT (if appropriate), similar to the logic used in src/cmake/externalpackages.cmake.

Suggested change

set(CUDA_TOOLKIT_ROOT_DIR ${CUDAToolkit_ROOT_DIR})

# Derive a usable CUDA toolkit root directory from CUDAToolkit variables.

set(CUDA_TOOLKIT_ROOT_DIR "")

if (CUDAToolkit_LIBRARY_ROOT)

# Use the library root directly when available.

set(CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_LIBRARY_ROOT}")

elseif (CUDAToolkit_BIN_DIR)

# Fall back to the parent of the bin directory.

get_filename_component(CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_BIN_DIR}" DIRECTORY)

elseif (CUDAToolkit_ROOT)

# As a last resort, use the generic CUDAToolkit root, if defined.

set(CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_ROOT}")

endif ()

Copilot · 2026-02-28T03:03:00Z

+    add_custom_command(OUTPUT ${cuda_ptx}
+        COMMAND ${CUDA_NVCC_EXECUTABLE}
+                ${_ccp_OPTIONS}
+                -ptx


cuda_compile_ptx() invokes ${CUDA_NVCC_EXECUTABLE}, but CUDAToolkit_NVCC_EXECUTABLE may be unset on systems that have CUDA runtime libraries/headers but no nvcc installed. Add a configure-time fallback/check (e.g., find_program(CUDA_NVCC_EXECUTABLE NAMES nvcc ...) and message(FATAL_ERROR ...) if still not found) so this fails during configuration rather than at build time.

Copilot · 2026-02-28T03:03:00Z

+        if (CUDAToolkit_LIBRARY_ROOT)
+            set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_LIBRARY_ROOT}")
+        elseif (CUDAToolkit_TARGET_DIR)
+            set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_TARGET_DIR}")
+        elseif (CUDAToolkit_BIN_DIR)
+            get_filename_component (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_BIN_DIR}" DIRECTORY)


CUDA_TOOLKIT_ROOT_DIR is later treated as a full toolkit root (used for --cuda-path=..., .../bin nvcc lookup, and .../lib64 paths). Setting it from CUDAToolkit_LIBRARY_ROOT can be inconsistent with those uses on toolkits where the library root is not the same as the toolkit root. Consider preferring CUDAToolkit_TARGET_DIR (or deriving from CUDAToolkit_BIN_DIR) for CUDA_TOOLKIT_ROOT_DIR, and using CUDAToolkit_LIBRARY_ROOT only for library searches.

Suggested change

if (CUDAToolkit_LIBRARY_ROOT)

set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_LIBRARY_ROOT}")

elseif (CUDAToolkit_TARGET_DIR)

set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_TARGET_DIR}")

elseif (CUDAToolkit_BIN_DIR)

get_filename_component (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_BIN_DIR}" DIRECTORY)

if (CUDAToolkit_TARGET_DIR)

set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_TARGET_DIR}")

elseif (CUDAToolkit_BIN_DIR)

get_filename_component (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_BIN_DIR}" DIRECTORY)

elseif (CUDAToolkit_LIBRARY_ROOT)

# Fallback: library root may not always equal the full toolkit root,

# but is used here as a last-resort compatibility measure.

set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_LIBRARY_ROOT}")

lgritz · 2026-02-28T03:06:43Z

@MarkLeone Before I merged this, I asked GitHub Copilot for a review, and it had a few comments. Do you have any thoughts on this? Ignore? Accept? Or do you want to amend on your own in some way?

(Also, I rebased your patch on top of the current main to make sure there were no conflicts.)

lgritz · 2026-02-28T03:08:59Z

Also, I asked Claude Code to take a whack at it, and its reply was:

⏺ Now I have enough context. Here's my critique:

PR #2073 Review: Switch from FindCUDA to FindCUDAToolkit

Overall: The motivation is sound — FindCUDA has been deprecated since CMake 3.27, and
cmake_minimum_required is already 3.19. The general approach (migrate the package find, add a
compatibility bridge for legacy variables) is reasonable. However, there are a few concrete bugs
and design issues.

Bug: CUDAToolkit_ROOT_DIR doesn't exist

In testsuite/example-cuda/CMakeLists.txt:
set(CUDA_TOOLKIT_ROOT_DIR ${CUDAToolkit_ROOT_DIR})
FindCUDAToolkit does not export CUDAToolkit_ROOT_DIR. The documented output variables are
CUDAToolkit_LIBRARY_ROOT, CUDAToolkit_TARGET_DIR, and CUDAToolkit_BIN_DIR. This line silently sets
CUDA_TOOLKIT_ROOT_DIR to empty.

The downstream find_library calls for CUDA_nvrtc_LIBRARY and CUDA_cuda_LIBRARY use HINTS
${CUDA_TOOLKIT_ROOT_DIR}, so they lose their hints and fall back to system paths — which may or
may not work. The imported-target overrides (CUDA::nvrtc, CUDA::cuda_driver) partially paper over
this, but only when those targets exist.

The same derivation logic that externalpackages.cmake uses (CUDAToolkit_LIBRARY_ROOT →
CUDAToolkit_TARGET_DIR → CUDAToolkit_BIN_DIR parent) should be applied here too, or at minimum use
CUDAToolkit_LIBRARY_ROOT.

Fragile: CUDA_INCLUDES silently truncated to first element

set (CUDA_INCLUDES ${CUDAToolkit_INCLUDE_DIRS})
if (CUDA_INCLUDES)
list (GET CUDA_INCLUDES 0 CUDA_INCLUDES)
endif ()

CUDAToolkit_INCLUDE_DIRS is a list, but cuda_macros.cmake uses it as a single string via
"-I${CUDA_INCLUDES}" — so the truncation is load-bearing. The problem is it's silent and
undocumented. On cross-compilation targets where the toolkit has multiple include dirs, this will
silently produce broken builds.

Either document why only the first element is valid here, or better yet, fix cuda_macros.cmake to
handle a list (i.e., use list(TRANSFORM ... PREPEND -I) like it already does for IMATH_INCLUDES).

Concern: CUDA_EXTRA_LIBS redundancy with imported targets

set(CUDA_EXTRA_LIBS ${CUDA_EXTRA_LIBS} dl rt)

When cudart_lib is CUDA::cudart (an imported target), dl and rt should be carried as transitive
dependencies automatically. Adding them explicitly to CUDA_EXTRA_LIBS is redundant. It's harmless
most of the time but could cause link-order surprises and muddies the intent.

Minor: CUDA_LIB_FLAGS relies on correctly-derived CUDA_TOOLKIT_ROOT_DIR

cuda_macros.cmake uses ${CUDA_LIB_FLAGS} which is --cuda-path=${CUDA_TOOLKIT_ROOT_DIR}. The
derivation logic is:

if (CUDAToolkit_LIBRARY_ROOT)
set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_LIBRARY_ROOT}")
elseif (CUDAToolkit_TARGET_DIR)
set (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_TARGET_DIR}")
elseif (CUDAToolkit_BIN_DIR)
get_filename_component (CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_BIN_DIR}" DIRECTORY)
endif ()

On cross-compilation setups, CUDAToolkit_LIBRARY_ROOT can be the sysroot-relative target lib dir,
not the host toolkit root. Using it as --cuda-path would point clang/nvcc at the wrong place. This
is a pre-existing fragility, but it's worth noting that the derivation logic assumes a
non-cross-compile host layout.

Minor: testing.cmake relies on CUDA_TOOLKIT_ROOT_DIR

src/cmake/testing.cmake:91 sets LD_LIBRARY_PATH=${CUDA_TOOLKIT_ROOT_DIR}/lib64. This is fine as
long as the derivation succeeds, but if it fails (the FATAL_ERROR guard is good), tests break.
This path was already a concern in the original, so no regression here.

What's good

The 4-way cudart_lib selection (prefer CUDA::cudart_static/CUDA::cudart imported targets, fall
back to manual find_library) is well-structured.
Removing include_directories(BEFORE "${CUDA_INCLUDES}") in favor of target-level
target_include_directories in osl_optix_target is a proper modernization.
The cuda_compile_ptx reimplementation in example-cuda is clean; the callers already pass
-I${CUDA_INCLUDE_DIRS} explicitly so the omission of implicit includes from the helper is not a
regression.
The stale "Is it really a good idea?" comment being dropped is appropriate — the concern is now
resolved by using imported targets.
cmake_minimum_required(VERSION 3.19) bump in example-cuda correctly aligns it with the top-level
project.

lgritz · 2026-02-28T03:21:33Z

BTW, I am neither particularly promoting nor eschewing these tools, but I do think it's instructive to give them a spin and see what they add to code review -- how much of the time it catches important things we miss, versus how much it's just noise.

lgritz · 2026-03-20T02:36:59Z

@MarkLeone You out there? No particular rush, but before I merged this I did want your feedback on the Copilot comments.

MarkLeone · 2026-04-06T21:21:50Z

Sorry for the delayed reply. I'll take a look.

tgrant-nv · 2026-10-08T22:01:13Z

This change seems to be mostly complementary to #2181, but is also a bit out of date. So I think the best course might be to merge (or not merge 😉) #2181 on its own merits, and to close this PR and to reapply its changes (in spirit, if not in exact form) on top of main.

This PR replaces use of the deprecated `FindCUDA` module with CMake's newer `FindCUDAToolkit`. This change is a spiritual successor to Mark's old PR #2073. The overall direction remains the same, but has been reworked on top of the current state of `main`. We're including some additional fixes discovered while testing the build on Windows. #### FindCUDAToolkit Migration The build now uses `checked_find_package(CUDAToolkit)` and CMake's imported CUDA runtime targets. The migration: * Uses the canonical `CUDAToolkit_*` variables internally, while continuing to accept `CUDA_TOOLKIT_ROOT_DIR` as a legacy fallback input. * Links `CUDA::cudart` or `CUDA::cudart_static` rather than manually locating the CUDA runtime and appending Unix-specific system libraries. * Passes the complete CUDA toolkit include list to NVCC and Clang. * Adds the relocated CUDA C++ Core Libraries (`include/cccl`) when necessary for CUDA 13 and older supported CMake versions. * Updates the remaining build and test logic to use `CUDAToolkit_FOUND`, `CUDAToolkit_VERSION`, and purpose-specific OSL variables rather than recreating the old `FindCUDA` result variables. * Uses platform-aware CTest runtime paths so CUDA and NVRTC shared libraries can be found on both Windows and Unix-like systems. * Uses `VERBATIM` custom commands so paths containing spaces are handled correctly. #### Windows Device Alignment Fix Testing on Windows revealed a separate incompatibility in the CUDA renderer support library. When building with older MSVC versions, Clang CUDA builds can resolve `__builtin_assume_aligned` to an MSVC host-only declaration, producing an error when it is used in device code. We added a typed `device_assume_aligned` helper. It uses `__builtin_assume` to express the alignment guarantee on Windows and retains `__builtin_assume_aligned` on other platforms. The helper preserves the pointer type and evaluates each argument only once. #### Standalone CUDA Example We also migrated the standalone `example-cuda` project to `FindCUDAToolkit`. The build now reaches CUDA source compilation but encounters a pre-existing use of the now-defuct `OSL/device_string.h` API. Updating the example to OSL's current hash-based device string interface will be handled in a separate PR. The removed `cuda_compile_ptx` helper is replaced with a local NVCC custom command, and the example links directly against the `CUDA::cudart`, `CUDA::nvrtc`, and `CUDA::cuda_driver` imported targets. The example now propagates its complete dependency include paths into PTX compilation and uses `sm_75` as its default architecture with CUDA 13, which no longer supports the previous architectures. Older CUDA versions retain an `sm_60` default, and `CUDA_TARGET_ARCH` may still be overridden explicitly. ### Tests We did not add any new tests, and we didn't observe any change in the test results relative to the baseline. Linux configurations were exercised with CUDA 12.8 through 13.3 and LLVM 15.0.7 through 22.1.8, including the following combinations: * CUDA 12.8 / LLVM 15.0.7 * CUDA 12.9 / LLVM 18.1.0 * CUDA 13.0 and 13.1 / LLVM 20.1.8 * CUDA 13.3 / LLVM 22.1.8 * CUDA 13.3 / LLVM 20.1.8 * CUDA 13.3 / LLVM 15.0.7 Additional configuration and build checks covered: * CMake 3.24.4 and 4.0.4. * Dynamic and static CUDA runtime targets. * The legacy `CUDA_TOOLKIT_ROOT_DIR` input. * CUDA 13's relocated CCCL headers with an older supported CMake. * CUDA/OptiX test runtime paths on Linux and Windows. * CUDA 12.4 and 13.3 installations on Windows. * CUDA installation paths containing spaces. * Loading cudart and NVRTC with an otherwise clean Windows `PATH`. Assisted-by: OpenAI Codex / GPT-6 --------- Signed-off-by: Tim Grant <tgrant@nvidia.com>

lgritz · 2026-10-16T23:59:56Z

Closing, replaced by #2185

build: Switch from FindCUDA to FindCUDAToolkit (AcademySoftwareFounda…

30c135a

…tion#2072) FindCUDA is obsolete. FindCUDAToolkit was added in CMake 3.17. Current cmake_minimum_required is already 3.19. Signed-off-by: Mark Leone <mleone@nvidia.com>

lgritz requested a review from Copilot February 28, 2026 02:58

Copilot started reviewing on behalf of lgritz February 28, 2026 02:59

View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

tgrant-nv mentioned this pull request Oct 15, 2026

Switch from FindCUDA to the FindCUDAToolkit #2185

Merged

5 of 6 tasks

lgritz closed this Oct 16, 2026

Navigation Menu

Repository navigation

build: Switch from FindCUDA to FindCUDAToolkit (#2072) - #2073

build: Switch from FindCUDA to FindCUDAToolkit (#2072)#2073
MarkLeone wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
MarkLeone:main

MarkLeone commented Feb 11, 2026

Uh oh!

linux-foundation-easycla Bot commented Feb 11, 2026 •

edited

Loading

Uh oh!

lgritz commented Feb 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

lgritz commented Feb 28, 2026

Uh oh!

lgritz commented Feb 28, 2026

Uh oh!

lgritz commented Feb 28, 2026

Uh oh!

lgritz commented Mar 20, 2026

Uh oh!

MarkLeone commented Apr 6, 2026

Uh oh!

tgrant-nv commented Oct 8, 2026

Uh oh!

lgritz commented Oct 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-set(CUDA_TOOLKIT_ROOT_DIR ${CUDAToolkit_ROOT_DIR})
+# Derive a usable CUDA toolkit root directory from CUDAToolkit variables.
+set(CUDA_TOOLKIT_ROOT_DIR "")
+if (CUDAToolkit_LIBRARY_ROOT)
+    # Use the library root directly when available.
+    set(CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_LIBRARY_ROOT}")
+elseif (CUDAToolkit_BIN_DIR)
+    # Fall back to the parent of the bin directory.
+    get_filename_component(CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_BIN_DIR}" DIRECTORY)
+elseif (CUDAToolkit_ROOT)
+    # As a last resort, use the generic CUDAToolkit root, if defined.
+    set(CUDA_TOOLKIT_ROOT_DIR "${CUDAToolkit_ROOT}")
+endif ()

Conversation

MarkLeone commented Feb 11, 2026

Uh oh!

linux-foundation-easycla Bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgritz commented Feb 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

lgritz commented Feb 28, 2026

Uh oh!

lgritz commented Feb 28, 2026

Uh oh!

lgritz commented Feb 28, 2026

Uh oh!

lgritz commented Mar 20, 2026

Uh oh!

MarkLeone commented Apr 6, 2026

Uh oh!

tgrant-nv commented Oct 8, 2026

Uh oh!

lgritz commented Oct 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

linux-foundation-easycla Bot commented Feb 11, 2026 •

edited

Loading