-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Simplified CUDA language detection in cmake #10564
Conversation
|
It seems that it's failing due to not picking up MSHADOW_USE_F16C on windows, which should be accoring to dmlc/mshadow@0b4cedd @marcoabreu can i clean and restart the job somehow without empty triggering commits? |
cjolivier01
left a comment
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 would need to be a CI that checked both USE_OLDCMAKECUDA on and off
|
@lebeg I granted you the permission |
|
dmlc/mshadow#333 fix fp16 with msvc |
|
But the CMakefile doesn't set F16C for MSVC. https://github.com/dmlc/mshadow/blob/0b4cedd7015cc69191f8338a8feaacda90697758/cmake/mshadow.cmake#L51 |
|
@rahul003 because when use first_cuda.the cmake/mshadow.cmake not include. |
|
@cjolivier01 can you take another look? |
| mxnet_option(USE_SIGNAL_HANDLER "Print stack traces on segfaults." OFF) | ||
|
|
||
| if(USE_CUDA AND NOT USE_OLDCMAKECUDA) | ||
| message(STATUS "CMake version '${CMAKE_VERSION}' using generator '${CMAKE_GENERATOR}'") |
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.
can we leave the diagnostics about cmake version and generator?
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 moved it to log unconditionally at the top of the file
|
LGTM |
|
@cjolivier01 ping again |
|
@lebeg @cjolivier01 can we get this merged? |
|
There are still problems for the windows build for some reason. |
|
windows build is some problem. |
|
I will look into it as soon as I get some time. |
|
@lebeg - ping |
|
@lebeg - bounce |
yajiedesign
left a comment
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.
please fix windows build.
|
@lebeg - any update? |
|
Kenapa litecoin tidak bisa masuk |
|
Any update on the PR @lebeg |
|
@sandeep-krishnamurthy Please change label to pr-awaiting-testing |
|
This PR is already a bit old and now it depends on #12331. I will take a look at updating it as soon as possible. |
|
@lebeg Can you resolve conflicts? |
|
@nswamy @sandeep-krishnamurthy the PR seems to be older than a month. can you please close it. @lebeg feel free to reopen the PR once the changes are ready |
Description
The version checking of cmake was buggy and confusing.
Checklist
Essentials
Changes