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

[cmake] switch to FindCUDAToolkit #6457

Merged
merged 1 commit into from
May 17, 2024

Conversation

characat0
Copy link
Contributor

Cmake 3.17 introduced FindCUDAToolkit and deprecated FindCUDA, when cross compiling on linux, we found some weird behaviour, like having to run two or even three configure steps (JuliaPackaging/Yggdrasil#8593 with LightGBM for Julia) (JuliaPackaging/Yggdrasil#4554 (comment), Torch had the same issue).
Switching to FindCUDAToolkit should make this easier.

@jameslamb jameslamb changed the title [ci] switch to FindCUDAToolkit [cmake] switch to FindCUDAToolkit May 17, 2024
Copy link
Collaborator

@jameslamb jameslamb 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 so much for this! I strongly support this change.

As it says in https://cmake.org/cmake/help/latest/module/FindCUDA.html

To find and use the CUDA toolkit libraries manually, use the FindCUDAToolkit module instead [of FindCUDA]. It works regardless of the CUDA language being enabled.

@jameslamb
Copy link
Collaborator

Double-checked the build logs and this looks like it's working great.

...
-- The CUDA compiler identification is NVIDIA 11.8.89
-- Detecting CUDA compiler ABI info
-- Detecting CUDA compiler ABI info - done
-- Check for working CUDA compiler: /usr/local/cuda/bin/nvcc - skipped
-- Detecting CUDA compile features
-- Detecting CUDA compile features - done
...
-- Found CUDAToolkit: /usr/local/cuda/include (found suitable version "11.8.89", minimum required is "11.0") 
...
[2/74] Building CUDA object CMakeFiles/histo_16_64_256-fulldata_sp_const.dir/src/treelearner/kernels/histogram_16_64_256.cu.o
[3/74] Building CXX object CMakeFiles/lightgbm_objs.dir/src/boosting/prediction_early_stop.cpp.o
[4/74] Building CUDA object CMakeFiles/histo_16_64_256_sp.dir/src/treelearner/kernels/histogram_16_64_256.cu.o
[5/74] Building CUDA object CMakeFiles/histo_16_64_256_sp_const.dir/src/treelearner/kernels/histogram_16_64_256.cu.o
[6/74] Building CUDA object CMakeFiles/histo_16_64_256-fulldata_sp.dir/src/treelearner/kernels/histogram_16_64_256.cu.o
...

(build link)

Based on that, all the other passing CI, and my own understanding of FindCUDA() vs. FindCUDAToolkit(), I'm confident merging this.

@shiyu1994 if you come back to this and disagree please let me know and we can change / revert it before the release (#6439).

@jameslamb jameslamb merged commit 3e9ab53 into microsoft:master May 17, 2024
38 checks passed
@jameslamb
Copy link
Collaborator

@characat0 thanks again, come back and contribute any time!

@characat0
Copy link
Contributor Author

Thanks for remembering @jameslamb, always a pleasure

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

Successfully merging this pull request may close these issues.

None yet

2 participants