[libcu++] Remove cudart dependency from cuda_error - #11086
Conversation
694576a to
c2a87f6
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds centralized CUDA driver entry-point resolution and error handling. It removes legacy driver-call helpers, updates ChangesCUDA driver API
Suggested reviewers: Merge Risk: 🟡 Moderate · up to This PR removes the CUDA Runtime dependency from CUDA error handling, but the current implementation still has unresolved compile, link, and header-compatibility risks that could break Windows builds, standalone header consumers, or NVRTC-related builds. It is not merge-ready until these issues are addressed or explicitly accepted. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
libcudacxx/include/cuda/__driver/entry_point.h (1)
66-66: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winsuggestion: In the
_CCCL_OS(WINDOWS)branch,::GetProcAddressreturnsFARPROC, while__fnisvoid*. MSVC accepts this implicit conversion as an extension, but other Windows toolchains can reject it. Use an explicitreinterpret_cast<void*>to avoid relying on the extension.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f7e78a39-2e64-4f6e-a6c1-df26bb5f11c9
📒 Files selected for processing (3)
libcudacxx/include/cuda/__driver/driver_api.hlibcudacxx/include/cuda/__driver/entry_point.hlibcudacxx/include/cuda/std/__exception/cuda_error.h
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| static auto __driver_fn = _CCCLRT_GET_DRIVER_FUNCTION(cuCtxGetDevice); | ||
| ::CUdevice __result{}; | ||
| ::cuda::__driver::__call_driver_fn(__driver_fn, "Failed to get current context", &__result); | ||
| _CCCL_TRY_DRIVER_API(__driver_fn, "Failed to get current context", &__result); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
suggestion: The message duplicates the one used by __ctxGetCurrent on line 244. This call queries the device of the current context, so the message misreports the failed operation.
Fix
- _CCCL_TRY_DRIVER_API(__driver_fn, "Failed to get current context", &__result);
+ _CCCL_TRY_DRIVER_API(__driver_fn, "Failed to get the device of the current context", &__result);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _CCCL_TRY_DRIVER_API(__driver_fn, "Failed to get current context", &__result); | |
| _CCCL_TRY_DRIVER_API(__driver_fn, "Failed to get the device of the current context", &__result); |
| template <::cudaError_t _Error> | ||
| [[noreturn]] _CCCL_HOST_API void __throw_cuda_error( | ||
| const char* __msg, | ||
| const char* __api = nullptr, | ||
| const ::cuda::std::source_location& __loc = ::cuda::std::source_location::current()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Confirm the definition sites of __throw_cuda_error and __init and the include edges between the three headers.
set -euo pipefail
fd -t f 'entry_point.h|driver_api.h|cuda_error.h' libcudacxx/include | while IFS= read -r f; do
echo "=== $f"
rg -n '^\s*#\s*include' "$f"
rg -n '__throw_cuda_error|__init\b' "$f"
doneRepository: NVIDIA/cccl
Length of output: 2783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== entry_point.h: relevant definitions and call paths'
sed -n '35,190p' libcudacxx/include/cuda/__driver/entry_point.h
echo '=== driver_api.h: __init definition and header guards'
sed -n '1,85p' libcudacxx/include/cuda/__driver/driver_api.h
echo '=== cuda_error.h: include guard and __throw_cuda_error definition'
sed -n '1,125p' libcudacxx/include/cuda/std/__exception/cuda_error.h
echo '=== direct consumers of entry_point.h and driver_api.h'
rg -n -C 2 '# *include <cuda(/std)?/__driver/(entry_point|driver_api)\.h>|__getProcAddressFn|__get_driver_entry_point(_no_init)?' libcudacxx/include libcudacxx/test 2>/dev/null || trueRepository: NVIDIA/cccl
Length of output: 32713
important: Provide definitions for __throw_cuda_error and __init to standalone users of entry_point.h. Calls through __getProcAddressFn, __get_driver_entry_point_no_init, or __get_driver_entry_point can emit unresolved references because this header provides only declarations, while the definitions are in cuda_error.h and driver_api.h.
| static void* __driver_library = ::dlopen(__driver_library_name, RTLD_NOW); | ||
| if (__driver_library == nullptr) | ||
| { | ||
| ::cuda::__throw_cuda_error<::cudaErrorUnknown>("Failed to load libcuda.so.1"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
suggestion: On Android the loaded library is libcuda.so, but the failure message always reports libcuda.so.1. Report the name that was actually used. The same applies to the dlsym message on line 85.
Fix
- ::cuda::__throw_cuda_error<::cudaErrorUnknown>("Failed to load libcuda.so.1");
+ ::cuda::__throw_cuda_error<::cudaErrorUnknown>("Failed to load the CUDA driver library");| # pragma system_header | ||
| #endif // no system header | ||
|
|
||
| #include <cuda/__driver/entry_point.h> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# 1) Show the guards around the cuda_error class. 2) Show the includes entry_point.h drags in.
set -euo pipefail
f=$(fd -t f 'cuda_error.h' libcudacxx/include | head -n1)
echo "=== $f"
rg -n '#\s*if|#\s*else|#\s*endif|#\s*include|class cuda_error|__getErrorString' "$f"
e=$(fd -t f 'entry_point.h' libcudacxx/include | head -n1)
echo "=== $e"
rg -n '#\s*if|#\s*include|WIN32_LEAN_AND_MEAN|NOMINMAX' "$e"Repository: NVIDIA/cccl
Length of output: 1772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== cuda_error.h: class and constructor branches'
sed -n '30,110p' libcudacxx/include/cuda/std/__exception/cuda_error.h
echo '=== entry_point.h: declarations and platform includes'
sed -n '20,115p' libcudacxx/include/cuda/__driver/entry_point.h
echo '=== entry_point.h: remaining scope and getErrorString references'
sed -n '115,205p' libcudacxx/include/cuda/__driver/entry_point.h
rg -n --glob '*.{h,hpp,cu,cpp,cuh}' '__getErrorString|_CCCL_HAS_CTK\(\)|_CCCL_COMPILER\(NVRTC\)' libcudacxx/include/cuda | head -120
echo '=== relevant macro definitions'
rg -n '_CCCL_(HAS_CTK|HOSTED|COMPILER|OS)' libcudacxx/include/cuda/std/detail/__config libcudacxx/include/cuda/__driver/entry_point.h | head -120Repository: NVIDIA/cccl
Length of output: 21840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== hosted/compiler macro definitions'
rg -n --glob '*.{h,hpp,inc}' \
'#[[:space:]]*define[[:space:]]+_CCCL_HOSTED|`#define`[[:space:]]+_CCCL_HOSTED|_CCCL_HOSTED\(\)|_CCCL_COMPILER\(NVRTC\)' \
libcudacxx/include/cuda libcudacxx/include/cuda/std/detail | head -160
echo '=== Windows header protections in the relevant include tree'
rg -n --glob '*.{h,hpp,inc}' \
'WIN32_LEAN_AND_MEAN|NOMINMAX|`#include`[[:space:]]*[<"]windows\.h[>"]' \
libcudacxx/include/cuda libcudacxx/include/cuda/std | head -120Repository: NVIDIA/cccl
Length of output: 14803
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== configuration candidates'
fd -t f '__config|config' libcudacxx/include | head -80
echo '=== macro definitions in configuration candidates'
files=$(fd -t f '__config|config' libcudacxx/include)
rg -n \
'#[[:space:]]*define[[:space:]]+_CCCL_(HOSTED|FREESTANDING)|_CCCL_HOSTED|_CCCL_COMPILER|NVRTC' \
$files | head -180Repository: NVIDIA/cccl
Length of output: 755
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== cuda/std/detail/__config'
sed -n '1,140p' libcudacxx/include/cuda/std/detail/__config
echo '=== cuda/__cccl_config'
sed -n '1,180p' libcudacxx/include/cuda/__cccl_config
echo '=== definitions in direct configuration area'
rg -n --glob '__config*' --glob '__cccl_config*' --glob '*.h' --glob '*.hpp' \
'#[[:space:]]*define[[:space:]]+_CCCL_(HOSTED|FREESTANDING)|_CCCL_HOSTED|_CCCL_COMPILER|NVRTC' \
libcudacxx/include | head -180Repository: NVIDIA/cccl
Length of output: 22075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== hosted and device-compilation definitions'
rg -n \
'#[[:space:]]*define[[:space:]]+_CCCL_(HOSTED|FREESTANDING|DEVICE_COMPILATION)|_CCCL_(HOSTED|FREESTANDING|DEVICE_COMPILATION)\(\)' \
libcudacxx/include/cuda/std/__cccl libcudacxx/include/cuda/std/__internal | head -160
echo '=== NVRTC host/device policy comments and conditionals'
rg -n -C 2 \
'NVRTC compiles device code only|NVRTC.*host|_CCCL_COMPILER\(NVRTC\).*_CCCL_HOSTED|_CCCL_HOSTED\(\).*NVRTC' \
libcudacxx/include/cuda/std/__cccl libcudacxx/include/cuda/std/__internal libcudacxx/include/cuda | head -120Repository: NVIDIA/cccl
Length of output: 7162
important: Protect the windows.h inclusion in entry_point.h. On hosted Windows builds with _CCCL_HAS_CTK() enabled, cuda_error.h exposes macros such as min and max to every including translation unit. Define WIN32_LEAN_AND_MEAN and NOMINMAX around the include, or avoid the full Windows header.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9c7c7b74-edfe-427c-9ed9-ac28be000fe1
📒 Files selected for processing (1)
libcudacxx/include/cuda/std/__exception/cuda_error.h
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.
| template <int _Error> | ||
| [[noreturn]] friend _CCCL_HOST_API void | ||
| __throw_cuda_error(const char* __msg, const char* __api, const ::cuda::std::source_location& __loc) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 --glob '*.{h,hpp,cpp,cxx,cu,cuh}' \
'\b__throw_cuda_error\b|_CCCL_TRY_DRIVER_API\b' libcudacxx
# Verify that a namespace-scope declaration is visible before this class.
# Then compile the nearest driver-wrapper error-path test with the supported toolchain.Repository: NVIDIA/cccl
Length of output: 33304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- cuda_error.h ---'
cat -n libcudacxx/include/cuda/std/__exception/cuda_error.h | sed -n '1,145p'
printf '%s\n' '--- entry_point.h includes and declaration ---'
cat -n libcudacxx/include/cuda/__driver/entry_point.h | sed -n '1,60p'
printf '%s\n' '--- driver_api.h includes and macro body ---'
cat -n libcudacxx/include/cuda/__driver/driver_api.h | sed -n '1,58p'
printf '%s\n' '--- direct declarations and include graph references ---'
rg -n -C 2 --glob '*.{h,hpp,cpp,cxx,cu,cuh}' \
'#[[:space:]]*include[[:space:]]*[<"]cuda/(std/__exception/cuda_error|__driver/entry_point)|\b__throw_cuda_error\b' \
libcudacxx/include/cudaRepository: NVIDIA/cccl
Length of output: 32824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- cudaError_t declarations ---'
rg -n -C 3 --glob '*.{h,hpp,cpp,cxx,cu,cuh}' \
'typedef[[:space:]]+enum[[:space:]]+cudaError|enum[[:space:]]+cudaError|cudaError_t' \
. | head -160
printf '%s\n' '--- all helper definitions and qualified calls ---'
rg -n -C 2 --glob '*.{h,hpp,cpp,cxx,cu,cuh}' \
'template[[:space:]]*<[^>]*(_Error|cudaError_t)|(::cuda::)?__throw_cuda_error' \
libcudacxx/include/cudaRepository: NVIDIA/cccl
Length of output: 17876
🌐 Web query:
CUDA Runtime API cudaError_t typedef enum cudaError definition cuda_runtime_api.h
💡 Result:
In the CUDA Runtime API, cudaError_t is a typedef for the enumeration type cudaError [1][2][3]. This definition is located within the header file cuda_runtime_api.h (or more generally, headers included by it, such as driver_types.h) [1][4]. The enumeration defines the various error codes that can be returned by CUDA Runtime API functions [3][5]. Developers typically use the cudaError_t type for variables intended to store or return these error codes [5]. The official CUDA documentation classifies this as part of the data types used by the CUDA Runtime [1][3].
Citations:
- 1: https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__TYPES.html
- 2: https://developer.download.nvidia.com/compute/DevZone/docs/html/C/doc/html/group__CUDART__TYPES_gf599e5b8b829ce7db0f5216928f6ecb6.html
- 3: https://developer.download.nvidia.com/compute/DevZone/docs/html/C/doc/html/group__CUDART__TYPES.html
- 4: https://docs.nvidia.com/cuda/archive/12.0.1/cuda-runtime-api/group__CUDART__TYPES.html
- 5: https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__ERROR.html
**important: Make the friend definition use ::cudaError_t to match the namespace-scope declaration. The current template <int _Error> defines a different hidden template, so driver-wrapper calls may link without a definition. **
| // We can't use ordinary cuda_error's constructor here, because it uses cuGetErrorString to get the error string which | ||
| // might be unavailable if we fail to dlopen the driver or query the cuGetProcAddress function. Instead, we hardcode | ||
| // several known error strings that are used in these APIs. | ||
| template <::cudaError_t _Error> |
There was a problem hiding this comment.
Does this need to be templated? I guess the intent is that we check at compile-time that it covers all the error messages we hardcoded, but I think we can instead use clangs [[clang::enable_if]] (https://clang.llvm.org/docs/AttributeReference.html#enable-if) to catch this, which is much cheaper.
| { | ||
| const char* __fn_name = "cuGetProcAddress_v2"; | ||
| # if _CCCL_OS(WINDOWS) | ||
| static auto __driver_library = ::LoadLibraryExA("nvcuda.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32); |
There was a problem hiding this comment.
| static auto __driver_library = ::LoadLibraryExA("nvcuda.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32); | |
| static const auto __driver_library = ::LoadLibraryExA("nvcuda.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32); |
(and the other statics here as well while we're in the area).
| auto __driver_fn = reinterpret_cast<decltype(::cuInit)*>( | ||
| ::cuda::__driver::__get_driver_entry_point_impl(__get_proc_addr_fn, "cuInit", 12, 0)); | ||
| ::cuda::__driver::__call_driver_fn(__driver_fn, "Failed to initialize CUDA Driver", 0); | ||
| const auto __driver_fn = |
There was a problem hiding this comment.
| const auto __driver_fn = | |
| static const auto __driver_fn = |
There was a problem hiding this comment.
This is intentional. __init is called only once, initializing a dummy static bool variable in __get_driver_entry_point
There was a problem hiding this comment.
Right but if __init fails, do we also want to reload the function? Or cache it here as well. With the current setup, if __init fails, then the next time it's called it will load the driver function from scratch, making it static will essentially cache the function load.
| || ::cuda::std::is_same_v<decltype(__cccl_try_driver_api_status), ::cudaError_t>); \ | ||
| if (static_cast<::cudaError_t>(__cccl_try_driver_api_status) != ::cudaSuccess) \ | ||
| { \ | ||
| _CCCL_THROW(::cuda::cuda_error, static_cast<::cudaError_t>(__cccl_try_driver_api_status), _MSG); \ |
There was a problem hiding this comment.
TBH, now that this is a macro, it may be useful to include the stringified call in the error message
| _CCCL_THROW(::cuda::cuda_error, static_cast<::cudaError_t>(__cccl_try_driver_api_status), _MSG); \ | |
| _CCCL_THROW(::cuda::cuda_error, static_cast<::cudaError_t>(__cccl_try_driver_api_status), #_FN "(" #__VA_ARGS__ "): " _MSG); \ |
| # if _CCCL_HOSTED() | ||
| //! @brief Gets the cuGetProcAddress function pointer. | ||
| template <class = void> | ||
| [[nodiscard]] _CCCL_PUBLIC_HOST_API inline auto __getProcAddressFn() -> decltype(cuGetProcAddress)* |
There was a problem hiding this comment.
| [[nodiscard]] _CCCL_PUBLIC_HOST_API inline auto __getProcAddressFn() -> decltype(cuGetProcAddress)* | |
| [[nodiscard]] _CCCL_PUBLIC_HOST_API auto __getProcAddressFn() -> decltype(cuGetProcAddress)* |
(inline is implicit from template)
| [[nodiscard]] _CCCL_PUBLIC_HOST_API inline void* __get_driver_entry_point_no_init( | ||
| const char* __name, [[maybe_unused]] int __major = 12, [[maybe_unused]] int __minor = 0) |
There was a problem hiding this comment.
| [[nodiscard]] _CCCL_PUBLIC_HOST_API inline void* __get_driver_entry_point_no_init( | |
| const char* __name, [[maybe_unused]] int __major = 12, [[maybe_unused]] int __minor = 0) | |
| [[nodiscard]] _CCCL_PUBLIC_HOST_API void* __get_driver_entry_point_no_init( | |
| const char* __name, int __major = 12, int __minor = 0) |
| return __fn; | ||
| } | ||
|
|
||
| [[nodiscard]] _CCCL_HOST_API inline const char* __getErrorString(::cudaError_t __error) |
There was a problem hiding this comment.
Nit: can we have a version of this that uses CUresult as well? Can just call this version and cast the argument
| //! | ||
| //! @throws @c cuda::cuda_error if the symbol cannot be obtained or the CUDA driver failed to initialize. | ||
| [[nodiscard]] _CCCL_PUBLIC_HOST_API inline void* | ||
| __get_driver_entry_point(const char* __name, [[maybe_unused]] int __major = 12, [[maybe_unused]] int __minor = 0) |
There was a problem hiding this comment.
| __get_driver_entry_point(const char* __name, [[maybe_unused]] int __major = 12, [[maybe_unused]] int __minor = 0) | |
| __get_driver_entry_point(const char* __name, int __major = 12, int __minor = 0) |
2 participants
Currently, we use CUDA Runtime APIs in
cuda_errorwhich makes life more difficult for some of our customers, because it introduces an unwanted dependency.The function we use is
cudaGetErrorStringthat has a driver API equivalent. This PR refactors the code, so we can usecuGetErrorStringinstead.However, this comes with some non-trivial issues. In our machinery that obtains the function entry points for us, we throw
cuda_errorif something goes wrong. That one would usecuGetErrorStringto get the error message, which would try to get the entry point, that would... There is a circular dependency.To work around this problem, I hardcoded several error messages to our headers, so we avoid the
cuGetErrorStringcall in those cases.This PR also fixes uses of
_CCCL_TRY_CUDA_APIin our driver APIs, that would introduce a dependency on CUDA Runtime again, because it callscudaGetLastErrorwhen cleaning the error state.