-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Migrate to FindCUDAToolkit.cmake #3004
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e254b70
replaces cmake CUDA finder with FindCUDAToolkit
TinyTinni 5f94ad5
increase cmake version to 3.17
TinyTinni 9da5c52
re-introduce cuda test compile
TinyTinni d678073
links cudnn tests against cuda runtime
TinyTinni 426c93d
add cudatoolkit root for test compilation
TinyTinni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
cmake_minimum_required(VERSION 3.8.0) | ||
cmake_minimum_required(VERSION 3.17.0) | ||
|
||
project(dlib_project) | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
|
||
cmake_minimum_required(VERSION 3.8.0) | ||
cmake_minimum_required(VERSION 3.17.0) | ||
|
||
add_subdirectory(${CMAKE_CURRENT_LIST_DIR} dlib_build) | ||
|
2 changes: 1 addition & 1 deletion
2
dlib/cmake_utils/check_if_avx_instructions_executable_on_host.cmake
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
dlib/cmake_utils/check_if_sse4_instructions_executable_on_host.cmake
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,15 @@ | ||
|
||
cmake_minimum_required(VERSION 3.8.0) | ||
cmake_minimum_required(VERSION 3.17.0) | ||
project(cuda_test) | ||
|
||
include_directories(../../cuda) | ||
add_definitions(-DDLIB_USE_CUDA) | ||
|
||
# Override the FindCUDA.cmake setting to avoid duplication of host flags if using a toolchain: | ||
option(CUDA_PROPAGATE_HOST_FLAGS "Propage C/CXX_FLAGS and friends to the host compiler via -Xcompile" OFF) | ||
find_package(CUDA 7.5 REQUIRED) | ||
find_package(CUDAToolkit 7.5 REQUIRED) | ||
set(CUDA_HOST_COMPILATION_CPP ON) | ||
list(APPEND CUDA_NVCC_FLAGS "-arch=sm_50;-std=c++14;-D__STRICT_ANSI__;-D_MWAITXINTRIN_H_INCLUDED;-D_FORCE_INLINES") | ||
|
||
cuda_add_library(cuda_test STATIC cuda_test.cu ) | ||
enable_language(CUDA) | ||
add_library(cuda_test STATIC cuda_test.cu ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
# | ||
|
||
|
||
cmake_minimum_required(VERSION 3.8.0) | ||
cmake_minimum_required(VERSION 3.17.0) | ||
project(cblas) | ||
|
||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This won't work when generating pkgconfig files.
If you look at my old PR #2912, which included using FindCUDAToolkit.cmake, I was unable to properly interoperate with pkgconfig due to cmake's use of "blah::blah" when linking libraries. This was my final hurdle but sadly couldn't overcome it. It included a whole bunch of other tidy-ups too... Sad
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.
Oh no, this sounds bad. I wasn't reading all of the PR, because it is way too long. But I assume you reference this comment [1] of yours.
You suggest using the "old" cmake way of using XXX_LIBRARIES and XXX_INCLUDE_DIRS.
Given the result variables from the new finder [2], the major problem seems to be the XXX_LIBRARIES variable, which is not set. I was also browsing the cmake source code of the finder and there isn't even a private one.
Did you try out to get all the link libraries via a target property? [3] Maybe creating the XXX_LIBRARIES manually by extracting all the libraries from the LINK_LIBRARIES [4] property is fine.
[1] #2912 (comment)
[2] https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html#result-variables
[3] get_target_property: https://cmake.org/cmake/help/latest/command/get_target_property.html
[4] LINK_LIBRARIES: https://cmake.org/cmake/help/latest/prop_tgt/LINK_LIBRARIES.html
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 tried extracting all the link libraries but it isn't trivial. There are some old cmake libraries that attempt to do this but they're not maintained. Personally my preference would be to drop support for pkgconfig then this problem goes away. There is a ticket open https://gitlab.kitware.com/cmake/cmake/-/issues/22621 that discusses adding support for exporting pkgconfig files natively. But nothing's happened. Realistically, how many users are using dlib via pkgconfig? My guess is not very many.
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.
The issue with the link libraries is that you have to do it recursively for all dependencies. It can get a bit nasty.
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.
Thanks, this sounds at least like a solution if pkgconfig cannot be dropped.
Long-term wise, a solution is probably needed anyway, as CMake linking on targets will very likely stay.