-
Notifications
You must be signed in to change notification settings - Fork 27
Modern CMake and Eigen update #3
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,93 +1,41 @@ | ||
| ######### CMake Version ##################### | ||
| cmake_minimum_required(VERSION 2.8.11) | ||
| ############################################# | ||
| project(eigen-cuda LANGUAGES CXX) | ||
| cmake_minimum_required(VERSION 3.14) | ||
|
|
||
| ######### Options ########################### | ||
| option( CORE_USE_CUDA "Use CUDA to speed up certain parts of the code." ON ) | ||
| ############################################# | ||
| option(USE_CUDA "Whether to use CUDA" OFF) | ||
|
|
||
|
|
||
| ######### CUDA decisions #################### | ||
| if (CORE_USE_CUDA) | ||
| MESSAGE( STATUS ">> -------------- USING CUDA --------------" ) | ||
| set( CUDA_TOOLKIT_ROOT_DIR "/opt/cuda" ) | ||
| if (APPLE OR UNIX) | ||
| set(CMAKE_C_COMPILER /opt/cuda/bin/gcc) | ||
| set(CMAKE_CXX_COMPILER /opt/cuda/bin/g++) | ||
| elseif (WIN32) | ||
| ### By default we use VS | ||
| MESSAGE( STATUS ">> User compiler: MSVC" ) | ||
| MESSAGE( STATUS ">> Choosing a different compiler is not yet implemented for Windows" ) | ||
| endif() | ||
| endif() | ||
| ############################################# | ||
|
|
||
|
|
||
| ######### Info ############################## | ||
| MESSAGE( STATUS ">> CMAKE_C_COMPILER: " ${CMAKE_C_COMPILER} ) | ||
| MESSAGE( STATUS ">> CMAKE_CXX_COMPILER: " ${CMAKE_CXX_COMPILER} ) | ||
| ############################################# | ||
|
|
||
|
|
||
| ######### Project Name ###################### | ||
| project(eigencuda) | ||
| SET( EXECUTABLE_NAME run ) | ||
| ############################################# | ||
|
|
||
|
|
||
| ### Find includes in corresponding build directories | ||
| set( CMAKE_INCLUDE_CURRENT_DIR ON ) | ||
| ######### Have the binary placed into the source head | ||
| set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_SOURCE_DIR} ) | ||
| ### Output paths for multi-config builds (e.g. msvc) | ||
| foreach( OUTPUTCONFIG ${CMAKE_CONFIGURATION_TYPES} ) | ||
| string( TOUPPER ${OUTPUTCONFIG} OUTPUTCONFIG ) | ||
| set( CMAKE_RUNTIME_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_SOURCE_DIR} ) | ||
| # set( CMAKE_LIBRARY_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${youroutputdirectory} ) | ||
| # set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${youroutputdirectory} ) | ||
| endforeach( OUTPUTCONFIG CMAKE_CONFIGURATION_TYPES ) | ||
| ############################################# | ||
| set( CMAKE_DISABLE_SOURCE_CHANGES ON ) | ||
| set( CMAKE_DISABLE_IN_SOURCE_BUILD ON ) | ||
|
mhubii marked this conversation as resolved.
|
||
| ############################################# | ||
|
|
||
|
|
||
| ######### CUDA decisions #################### | ||
| if (CORE_USE_CUDA) | ||
| find_package(CUDA REQUIRED) | ||
| if (USE_CUDA) | ||
| enable_language(CUDA) | ||
| add_definitions(-DUSE_CUDA) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you'd like to modernise the cmake here, you could place this further below with the (also, I believe this
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added On the other note: The language has to be enabled or Eigen won't compile with cuda support. Not sure how to do this differently
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not suggesting to remove it, but it seems like it should be fine to place into the if-block further down, Or do you mean |
||
| endif() | ||
| ############################################# | ||
|
|
||
| ############################################# | ||
| set(SOURCE_FILES | ||
| src/test.cpp | ||
| src/kernel.cu | ||
| src/kernel.cpp | ||
| include(FetchContent) | ||
| FetchContent_Declare( | ||
| Eigen3 | ||
| GIT_REPOSITORY git@gitlab.com:libeigen/eigen.git | ||
| GIT_TAG master | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about fetching a branch instead of a commit or tag. It will mean people will get varying results over time. You say
But this would change the Eigen version for people over time, without any relation to the CUDA version -- assuming an old CUDA version I believe this example should still build, even on newer CMake. Newer CUDA versions will require newer Eigen versions, but unfortunately it is also my experience that newer Eigen versions break on older CUDA. Unfortunately, I don't see a way to future-proof this except maybe using conan or vcpkg, which seems a bit overkill...
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. generally agreed that using a tag / hash is better than a plain branch. Is there a table that maps cuda / eigen versions somehow?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not aware of one and I think it would be overkill to implement such a mapping here. |
||
| ) | ||
|
|
||
| set(HEADER_FILES | ||
| include/test.hpp | ||
| include/kernel.hpp | ||
| FetchContent_MakeAvailable(Eigen3) | ||
|
|
||
| if (USE_CUDA) | ||
| add_library(test | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src/kernel.cu | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src/test.cpp | ||
| ) | ||
| set_property(TARGET test | ||
| PROPERTY CUDA_SEPARABLE_COMPILATION ON | ||
| ) | ||
| else () | ||
| add_library(test | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src/kernel.cpp | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/src/test.cpp | ||
| ) | ||
| endif () | ||
|
|
||
| target_include_directories(test | ||
| PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include | ||
| ) | ||
| ############################################# | ||
|
|
||
|
|
||
|
|
||
| ############################################# | ||
| if (CORE_USE_CUDA) | ||
| include_directories( ${EXECUTABLE_NAME} PUBLIC ${PROJECT_SOURCE_DIR}/include ${PROJECT_SOURCE_DIR}/thirdparty) | ||
| cuda_add_executable( ${EXECUTABLE_NAME} main.cpp ${SOURCE_FILES} ) | ||
| else() | ||
| add_executable( ${EXECUTABLE_NAME} main.cpp ${SOURCE_FILES} ) | ||
| endif() | ||
|
|
||
| target_link_libraries( ${EXECUTABLE_NAME} ${CUDA_LIBRARIES}) | ||
|
|
||
| target_include_directories( ${EXECUTABLE_NAME} PUBLIC ${PROJECT_SOURCE_DIR}/include) | ||
| target_include_directories( ${EXECUTABLE_NAME} PRIVATE ${PROJECT_SOURCE_DIR}/thirdparty) | ||
| target_link_libraries(test Eigen3::Eigen) | ||
|
|
||
| set_property(TARGET ${EXECUTABLE_NAME} PROPERTY CXX_STANDARD 11) | ||
| set_property(TARGET ${EXECUTABLE_NAME} PROPERTY CXX_STANDARD_REQUIRED ON) | ||
| set_property(TARGET ${EXECUTABLE_NAME} PROPERTY CXX_EXTENSIONS OFF) | ||
| ############################################# | ||
| add_executable(main main.cpp) | ||
| target_link_libraries(main test) | ||
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
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.
If you remove this section, the build does not match the docs, which say
(on msvc, but also if you implement my comments on the readme)
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 okay, running Ubuntu here, not sure I can look into this
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.
why did you remove the section placing the binary into the root cmake dir?