-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[CUDA] added support for Clang #3886
Conversation
197de80
to
4df76bc
Compare
4df76bc
to
f8e435d
Compare
else() | ||
PROJECT(lightgbm LANGUAGES C CXX) | ||
endif() | ||
PROJECT(lightgbm LANGUAGES C CXX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMAKE_CUDA_HOST_COMPILER
should be set before PROJECT
(with CUDA
language) command but CMAKE_CXX_COMPILER
is available after PROJECT
command execution.
Once the CUDA language is enabled, the
CMAKE_CUDA_HOST_COMPILER
variable is read-only and changes to it are undefined behavior.
https://cmake.org/cmake/help/latest/variable/CMAKE_CUDA_HOST_COMPILER.html
And CMake stores path to that default compiler executable in the
CMAKE_C_COMPILER
variable.
https://stackoverflow.com/a/63944545
@@ -117,6 +113,8 @@ else() | |||
endif(USE_MPI) | |||
|
|||
if(USE_CUDA) | |||
SET(CMAKE_CUDA_HOST_COMPILER "${CMAKE_CXX_COMPILER}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested here way to set custom compiler via two lines
set(CMAKE_CUDA_HOST_COMPILER "${CMAKE_CXX_COMPILER}")
set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS} -ccbin ${CMAKE_CXX_COMPILER}")
results in
nvcc fatal : redefinition of argument 'compiler-bindir'
with old CUDA version. So I believe setting only CMAKE_CUDA_HOST_COMPILER
is enough:
This maps to the
nvcc -ccbin
option.
https://cmake.org/cmake/help/latest/variable/CMAKE_CUDA_HOST_COMPILER.html
docs/Installation-Guide.rst
Outdated
|
||
The following dependencies should be installed before compilation: | ||
|
||
- **CUDA** libraries. Please refer to `this detailed guide`_. | ||
- **CUDA** libraries. Please refer to `this detailed guide`_. Pay great attention to the minimum required versions of host compilers listed in the table from that guide and use only recommended versions of compilers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree! We're likely to make a mistake trying to replicate this all in CMakeLists.txt, and for the limited benefit of raising a more informative error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks! By the way I do have a laptop with an NVIDIA graphics card and I've been wanting to learn more about CUDA, so if there are ever small tasks I can help on please feel free to assign them to me. My primary focus for the near future will be on the Dask module, but happy to help with the CUDA stuff wherever I can.
@jameslamb Thank you very much! I think you can start from investigating why Dask works(?) fine on CUDA but fails on OpenCL. |
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Tried with all three CUDA versions we currently use at our CI. Everyone succeed.
9.0
10.0
11.2