Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CMakeLists to support newer CMake versions #1077

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Qiangong2
Copy link

@Qiangong2 Qiangong2 commented Feb 5, 2024

FIND_PACKAGE(CUDA) was deprecated starting with cmake 3.10 and flat out removed in cmake 3.27. For machines in environments with strict update requirements, this breaks compilation on newer systems.

FIND_PACKAGE(CUDAToolkit) works the exact same way, and is supported by cmake.

Thanks to the NERSC team for identifying the issue.

@biochem-fan
Copy link
Member

Thank you very much but can you make the pull request compatible with older versions of CMake?

@Qiangong2
Copy link
Author

CUDAToolkit was added in 3.10, and distributions as far back as RHEL/CentOS 7 all have at least 3.10 available in their repos. A user would need to be running RHEL 6 or older (which don't even support cmake 3.0) to not have CUDAToolkit support in their cmake install.

This pr essentially makes the requirement cmake 3.10 instead of cmake 3.0.

@biochem-fan
Copy link
Member

Isn't it just adding if-else with CMake version checks?

@FilipeMaia
Copy link

@Qiangong2, as @biochem-fan said, I would suggest suggest:

IF(CMAKE_VERSION VERSION_LESS 3.10)
FIND_PACKAGE(CUDA)
ELSE(CMAKE_VERSION)
# newer version of CMake use a different module name
FIND_PACKAGE(CUDAToolkit)
ENDIF(CMAKE_VERSION)

@Qiangong2
Copy link
Author

Added in the if/else statement as requested. I can verify it builds and runs without issue.

@biochem-fan
Copy link
Member

@Qiangong2 @FilipeMaia

Thank you very much.

I can verify it builds and runs without issue

Did you test on versions both before and after 3.10?
If so, I will merge this.

@Qiangong2
Copy link
Author

I don't have a machine old enough to test if it works before 3.10, but since the if statement would use FIND_PACKAGE(CUDA) if it returned true, I believe it is functional.

@biochem-fan
Copy link
Member

In FindCUDA, we can control arguments to nvcc via the CUDA_NVCC_FLAGS variable.
What is the new way to do this? We need to set the C++ standard and enable OpenMP in nvcc.
Failure to do so resulted in silent but serious problems before.

I looked at FindCUDA documentation:

It is no longer necessary to use this module or call find_package(CUDA) for compiling CUDA code. Instead, list CUDA among the languages named in the top-level call to the project() command, or call the enable_language() command with CUDA. Then one can add CUDA (.cu) sources directly to targets similar to other languages.

None of the pages linked from here mention CUDA_NVCC_FLAGS. Is the variable no longer supported after FindCUDA removal? What is the alternative way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants