Check for CUDA headers rather than compiler#28
Check for CUDA headers rather than compiler#28sethrj wants to merge 3 commits intoroot-project:masterfrom
Conversation
|
@amadio I see the following message: is there something I need to do? If it's because of the move to @root-project, then perhaps a CONTRIBUTING.md describing PR requirements would help. Thanks! 
|
This is just a message about commits requiring a GPG signature, I don't think you need to do anything. I will handle the merge when things are ready. The |
| endif() | ||
| endif() | ||
|
|
||
| # Print a pretty message if the caller doesn't have a FindVecGeom.cmake |
| # Print a pretty message if the caller doesn't have a FindVecGeom.cmake | ||
| include(FindPackageHandleStandardArgs) | ||
| set(${CMAKE_FIND_PACKAGE_NAME}_CONFIG "${CMAKE_CURRENT_LIST_FILE}") | ||
| find_package_handle_standard_args(@PROJECT_NAME@ CONFIG_MODE HANDLE_COMPONENTS) |
There was a problem hiding this comment.
It's probably better to hard-code VecCore in these lines, to ensure things still work when VecCore is used inside another project with add_subdirectory(). However, with ROOT deprecating VecCore, if this is fine for VecGeom/Celeritas, it can stay like this.
| set(VecCore_CUDA_FOUND True) | ||
| set(VecCore_CUDA_DEFINITIONS -DVECCORE_ENABLE_CUDA) | ||
| set(VecCore_CUDA_INCLUDE_DIR ${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}) | ||
| set(VecCore_CUDA_INCLUDE_DIR ${CUDAToolkit_INCLUDE_DIRS}) |
There was a problem hiding this comment.
Could you please explain the motivation for this change? The current way of doing things follows advice from CMake policy CMP0146, which suggests projects should use CMake's first class support for cuda via check_language(CUDA)/enable_language(CUDA). The addition of -DVECCORE_ENABLE_CUDA does require that the CUDA language be enabled if you are compiling CUDA code, because that will enable host/device macros, etc. If you want to handle that yourself, you can always find_package(VecCore) without specifying the CUDA component, then add_definitions(VECCORE_ENABLE_CUDA) yourself, without having to change this logic.
There was a problem hiding this comment.
For the record, this was last modified in this commit 743566f.
2 participants
Since VecCore is header-only, it needs only to check the CUDA toolkit location. This also avoids some odd side effects that occur with
check_language(which can interfere with variables inenable_language). Additionally, use standard reporting for the library being found when loaded viafind_package, and update the cmake to avoid warnings with newer versions.