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] CMake script for choosing CCs #75

Merged
merged 8 commits into from
Apr 16, 2020
Merged

Conversation

griwodz
Copy link
Member

@griwodz griwodz commented Feb 25, 2020

The idea is to have a CMake function that can be re-used and extended to get an up-to-date list of default CCs for compilation as CUDA develops further.

@griwodz
Copy link
Member Author

griwodz commented Feb 25, 2020

@simogasp Could you take a look at this attempt? It is not hooked into CMakeLists.txt yet, just to discuss the idea and link it to the bug report.

@griwodz griwodz mentioned this pull request Feb 25, 2020
@simogasp simogasp added cuda issues related to cuda versions scope:build labels Feb 25, 2020
@simogasp simogasp added this to the v1.0.0 milestone Feb 25, 2020
@griwodz
Copy link
Member Author

griwodz commented Feb 25, 2020

Would it be possible to upgrade to cmake 3.11 also in PopSift (and discard the commit "replace >=")?
I see that we use 3.11 in AliceVision, but not sure how to make it happen.

cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
@simogasp simogasp changed the base branch from develop to dev/cleaning February 25, 2020 13:48
@simogasp
Copy link
Member

simogasp commented Feb 25, 2020

I changed the destination base to dev/cleaning for the time being so we can see just the related diffs

cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
cmake/ChooseCudaCC.cmake Outdated Show resolved Hide resolved
@simogasp simogasp self-assigned this Feb 27, 2020
CMakeLists.txt Outdated Show resolved Hide resolved
else()
set(PopSift_CUDA_CC_LIST_BASIC 30 35 50 52 )
getFlagsForCudaCCList(PopSift_CUDA_CC_LIST
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess we want to give the possibility to the user to provide a -DPopSift_CUDA_CC_LIST=30;70 at cmake stage, am I correct?
In that case:

  1. we should document it in the README in the build section

  2. more importantly, we should check that those provided are good/supported. One way to go it could be something like:

# we always need to know which are all the supported CC
chooseCudaCC(PopSift_CUDA_CC_LIST_BASIC 
            PopSift_CUDA_GENCODE_FLAGS
            MIN_CC 30
            MIN_CUDA_VERSION 7.0)
# if PopSift_CUDA_CC_LIST is provided
if(DEFINED PopSift_CUDA_CC_LIST)
    # check each provided element is supported
    # ideally this function checks the provided cc are in the list generating an error if not
    check_if_supported(PopSift_CUDA_CC_LIST  PopSift_CUDA_CC_LIST_BASIC)
    getFlagsForCudaCCList(PopSift_CUDA_CC_LIST PopSift_CUDA_GENCODE_FLAGS)
else()
    set(PopSift_CUDA_CC_LIST ${PopSift_CUDA_CC_LIST_BASIC} CACHE STRING "CUDA CC versions to compile")
endif()
list(APPEND CUDA_NVCC_FLAGS "${PopSift_CUDA_GENCODE_FLAGS}")

and

function(check_if_supported PROVIDED_CC  SUPPORTED_CC)
    foreach(cc IN LISTS PROVIDED_CC)
        if(NOT ${cc} IN_LIST SUPPORTED_CC)
            message(FATAL_ERROR "Compute capability ${cc} not supported. Supported CC are ${SUPPORTED_CC}")
    endforeach()
endfunction()

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a really good thing? Without that check, a user can configure for a brand new CC that we haven't put into the ChooseCudaCC.cmake file yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you have a point!
We can then leave it like that and assume that it is an advanced feature and the user knows what is doing.

@simogasp
Copy link
Member

simogasp commented Mar 2, 2020

I was thinking, since we are going to use it in all the other projects, shall we rather create a repository with these sorts of help functions and import them as git submodules? @fabiencastan

@simogasp simogasp changed the base branch from dev/cleaning to develop April 12, 2020 16:00
@simogasp simogasp changed the title CMake script for choosing CCs [cmake] CMake script for choosing CCs Apr 12, 2020
@fabiencastan fabiencastan merged commit fadb397 into develop Apr 16, 2020
@fabiencastan fabiencastan deleted the dev/chooseCudaCC branch April 16, 2020 13:57
@simogasp simogasp linked an issue May 11, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda issues related to cuda versions scope:build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[build] debug broken?
3 participants