Conversation
There was a problem hiding this comment.
Pull request overview
Adds CUDA BLAS–related API coverage to the compatibility test suite, with Torch/Paddle behavioral differences captured via output placeholders to keep result files line-aligned for comparison.
Changes:
- Introduce
test/ops/CUDABlasTest.cppcoveringgetCurrentCUDABlasHandleandat::cuda::blas::gemm<T>(with Torch-side stubs when symbols aren’t linkable). - Extend CMake to locate CUDA Toolkit headers and add CUDA include paths to Torch/Paddle test targets.
- Add Paddle-specific compile/link adjustments for CUDA compat headers (define
PADDLE_WITH_CUDA, linkcudartwhen available).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
test/ops/CUDABlasTest.cpp |
New CUDA BLAS API tests plus Torch stub outputs for non-exported symbols |
cmake/build.cmake |
Adds Paddle CUDA compile definitions and links cudart when available |
CMakeLists.txt |
Detects CUDA Toolkit and threads CUDA include dirs into Torch/Paddle include paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Paddle's CUDA compat headers (CUDAContextLight.h, CUDAFunctions.h) | ||
| # require PADDLE_WITH_CUDA to be defined so that GPU type aliases | ||
| # (gpuStream_t, cudaDeviceProp) are resolved via cuda_runtime.h. | ||
| target_compile_definitions(${_test_name} PRIVATE PADDLE_WITH_CUDA) | ||
| # Link libcudart for CUDA runtime symbols used by the Paddle CUDA compat | ||
| # layer. | ||
| if(TARGET CUDA::cudart) | ||
| target_link_libraries(${_test_name} CUDA::cudart) | ||
| elseif(EXISTS "/usr/local/cuda/lib64/libcudart.so") | ||
| target_link_libraries(${_test_name} | ||
| "/usr/local/cuda/lib64/libcudart.so") | ||
| endif() |
There was a problem hiding this comment.
This unconditionally defines PADDLE_WITH_CUDA for all Paddle builds (USE_PADDLE_API=1), even if the detected Paddle installation is CPU-only or ROCm/HIP-based. That can force inclusion of CUDA-only types/headers and break compilation. Please gate PADDLE_WITH_CUDA (and the cudart linking) on actually having CUDA headers/libraries available (e.g., CUDAToolkit_FOUND / TARGET CUDA::cudart), or make the CUDA-dependent tests opt-in/out explicitly.
| # Paddle's CUDA compat headers (CUDAContextLight.h, CUDAFunctions.h) | |
| # require PADDLE_WITH_CUDA to be defined so that GPU type aliases | |
| # (gpuStream_t, cudaDeviceProp) are resolved via cuda_runtime.h. | |
| target_compile_definitions(${_test_name} PRIVATE PADDLE_WITH_CUDA) | |
| # Link libcudart for CUDA runtime symbols used by the Paddle CUDA compat | |
| # layer. | |
| if(TARGET CUDA::cudart) | |
| target_link_libraries(${_test_name} CUDA::cudart) | |
| elseif(EXISTS "/usr/local/cuda/lib64/libcudart.so") | |
| target_link_libraries(${_test_name} | |
| "/usr/local/cuda/lib64/libcudart.so") | |
| endif() | |
| # Only enable Paddle's CUDA pathways when CUDA is actually available. | |
| # This prevents defining PADDLE_WITH_CUDA for CPU-only or ROCm builds, | |
| # which would otherwise pull in CUDA-only headers/types and break | |
| # compilation. | |
| set(_paddle_cuda_available OFF) | |
| if(CUDAToolkit_FOUND) | |
| set(_paddle_cuda_available ON) | |
| elseif(TARGET CUDA::cudart) | |
| set(_paddle_cuda_available ON) | |
| elseif(EXISTS "/usr/local/cuda/include/cuda.h" | |
| AND EXISTS "/usr/local/cuda/lib64/libcudart.so") | |
| set(_paddle_cuda_available ON) | |
| endif() | |
| if(_paddle_cuda_available) | |
| # Paddle's CUDA compat headers (CUDAContextLight.h, CUDAFunctions.h) | |
| # require PADDLE_WITH_CUDA to be defined so that GPU type aliases | |
| # (gpuStream_t, cudaDeviceProp) are resolved via cuda_runtime.h. | |
| target_compile_definitions(${_test_name} PRIVATE PADDLE_WITH_CUDA) | |
| # Link libcudart for CUDA runtime symbols used by the Paddle CUDA | |
| # compat layer. | |
| if(TARGET CUDA::cudart) | |
| target_link_libraries(${_test_name} CUDA::cudart) | |
| elseif(EXISTS "/usr/local/cuda/lib64/libcudart.so") | |
| target_link_libraries(${_test_name} | |
| "/usr/local/cuda/lib64/libcudart.so") | |
| endif() | |
| else() | |
| message(WARNING | |
| "USE_PADDLE_API is enabled, but CUDA toolkit was not detected; " | |
| "skipping PADDLE_WITH_CUDA definition and cudart linkage for " | |
| "test target ${_test_name}.") | |
| endif() |
| "${CUDA_INCLUDE_DIRS}" | ||
| "${CUDA_INCLUDE_DIRS}/cccl") | ||
|
|
There was a problem hiding this comment.
CUDA_INCLUDE_DIRS is set to an empty string when CUDA headers aren’t found, but ${CUDA_INCLUDE_DIRS}/cccl will then expand to /cccl and be added as an include directory. Also, if CUDA_INCLUDE_DIRS is a CMake list (multiple include dirs), appending /cccl this way will only apply to the last element. Please guard this append (only add the cccl path when CUDA include dir is non-empty and exists) and derive the cccl subdir from a single concrete CUDA include path (e.g., the first element).
| "${CUDA_INCLUDE_DIRS}" | |
| "${CUDA_INCLUDE_DIRS}/cccl") | |
| "${CUDA_INCLUDE_DIRS}") | |
| # Append CUDA cccl include directory if available. | |
| if(CUDA_INCLUDE_DIRS) | |
| # CUDA_INCLUDE_DIRS may be a list; use the first element as the base path. | |
| list(GET CUDA_INCLUDE_DIRS 0 CUDA_INCLUDE_DIRS_FIRST) | |
| if(CUDA_INCLUDE_DIRS_FIRST AND EXISTS "${CUDA_INCLUDE_DIRS_FIRST}") | |
| list(APPEND PADDLE_INCLUDE_DIR "${CUDA_INCLUDE_DIRS_FIRST}/cccl") | |
| endif() | |
| endif() |
| message(WARNING "CUDA headers not found; CUDA tests may not compile.") | ||
| set(CUDA_INCLUDE_DIRS "") |
There was a problem hiding this comment.
find_package(CUDAToolkit QUIET) falls back to a warning and continues even when CUDA headers aren’t present, but test/ops/CUDABlasTest.cpp includes CUDA-specific headers and is always part of TEST_SRC_FILES, so the build can still hard-fail at compile time. Consider either making CUDA headers a hard requirement for building this test (use REQUIRED or a dedicated option) or conditionally excluding CUDABlasTest.cpp from TEST_SRC_FILES when CUDA isn’t found.
| message(WARNING "CUDA headers not found; CUDA tests may not compile.") | |
| set(CUDA_INCLUDE_DIRS "") | |
| message(STATUS | |
| "CUDA headers not found; CUDA-specific tests (e.g. CUDABlasTest) will be skipped.") | |
| set(CUDA_INCLUDE_DIRS "") | |
| # Avoid compiling CUDA-dependent tests when CUDA is not available. | |
| list(REMOVE_ITEM TEST_SRC_FILES | |
| "${PROJECT_SOURCE_DIR}/test/ops/CUDABlasTest.cpp") |
暂时无法对齐,先跳过