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

Use FindCUDAToolkit for cmake versions >= 3.17 #1124

Merged
merged 5 commits into from
Sep 6, 2023

Conversation

nilsfriess
Copy link
Collaborator

@nilsfriess nilsfriess commented Sep 5, 2023

As reported in #1119, FindCUDA is deprecated in recent cmake versions.
This PR adds our own FindCUDA that falls back to cmake's FindCUDA for cmake versions < 3.17. For cmake versions >= 3.17 it uses the replacement for FindCUDA, FindCUDAToolkit.

Fixes #1119

Copy link
Collaborator

@illuhad illuhad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is great to see!

I think nvc++ CI fails because it sets -DCUDA_TOOLKIT_ROOT_DIR to point it to nvc++'s CUDA installation.
I think it might make sense to continue to support CUDA_TOOLKIT_ROOT_DIR for backwards compatibility even when we use CUDAToolkit. Our documentation references it, and chances are user installations might also rely on it.

@nilsfriess
Copy link
Collaborator Author

Now it checks if CUDA_TOOLKIT_ROOT_DIR is defined and sets CUDAToolkit_ROOT to the value in CUDA_TOOLKIT_ROOT_DIR (unless CUDAToolkit_ROOT is itself defined)

@illuhad illuhad merged commit bb1fb38 into AdaptiveCpp:develop Sep 6, 2023
17 of 18 checks passed
@RedSoutherly
Copy link

Hello, I think this has caused a problem for me. This seems to make building with a CUDA backend on aarch64 linux fail. Looks like because the Cmake code is looking for the install dirs specific to a x64 machine. Currently working around by cloning the previous merge commit. But this could be an issue for other people developing on aarch64 systems. (Namely NVIDIA Jetson Developer Kits if you're curious about the use case).

@nilsfriess
Copy link
Collaborator Author

Thanks for reporting.
Can you try if manually passing -DCUDA_TOOLKIT_ROOT_DIR=/path/to/cuda to cmake solves the issue?

@tdavidcl
Copy link
Contributor

tdavidcl commented Sep 7, 2023

Hello, I think this has caused a problem for me. This seems to make building with a CUDA backend on aarch64 linux fail. Looks like because the Cmake code is looking for the install dirs specific to a x64 machine. Currently working around by cloning the previous merge commit. But this could be an issue for other people developing on aarch64 systems. (Namely NVIDIA Jetson Developer Kits if you're curious about the use case).

similar issue on debian (self-hosted runner) :

cmake \
    -DCMAKE_CXX_COMPILER=/usr/bin/clang++-15 \
    -DCLANG_EXECUTABLE_PATH=/usr/bin/clang++-15 \
    -DLLVM_DIR=/usr/lib/llvm-15/cmake \
    -DWITH_CUDA_BACKEND=ON -DWITH_ROCM_BACKEND=ON \
    -DWITH_LEVEL_ZERO_BACKEND=OFF \
    -DCUDA_TOOLKIT_ROOT_DIR=~/opt/cuda \
    -DROCM_PATH=/opt/rocm \
    -DCMAKE_INSTALL_PREFIX=../OpenSYCL_comp .

when running make :

Scanning dependencies of target rt-backend-cuda
make[2]: *** No rule to make target '/usr/lib/x86_64-linux-gnu/librt.so /home/docker/opt/cuda/lib64/stubs/libcuda.so', needed by 'src/runtime/librt-backend-cuda.so'.  Stop.
make[2]: *** Waiting for unfinished jobs....

in the docker image CUDA is installed like so :

RUN mkdir -p ~/opt/cuda
RUN wget -q -O cuda.sh http://developer.download.nvidia.com/compute/cuda/11.0.2/local_installers/cuda_11.0.2_450.51.05_linux.
RUN sudo sh ./cuda.sh --override --silent --toolkit --no-man-page --no-drm --no-opengl-libs --installpath=~/opt/cuda || true
RUN echo "CUDA Version ${{matrix.cuda}}" | sudo tee ~/opt/cuda/version.txt

this bit is the same as the OpenSYCL CI beside the install path

@nilsfriess
Copy link
Collaborator Author

Thanks, I was able to reproduce with cmake version 3.15. I thought we test with cmake version < 3.17 but maybe we actually don't...

Could you try if #1127 fixes it?

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.

CMake: Replace deprecated FindCUDA with FindCUDAToolkit
4 participants