build: Switch from FindCUDA to FindCUDAToolkit (#2072)

[MarkLeone]

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

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

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

[lgritz]

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]

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]

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]

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]

@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]

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]

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.