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_CUDA_ARCHITECTURES doesn't change when build-system invokes cmake #726

Merged

Conversation

robertmaynard
Copy link
Contributor

Consider the following:

cmake -DCMAKE_CUDA_ARCHITECTURES="" . #build for detected
touch <rmm_dir>/CMakeLists.txt
ninja #should be build for detected

cmake -DCMAKE_CUDA_ARCHITECTURES= . #build for all
touch <rmm_dir>/CMakeLists.txt
ninja #should be build for all

cmake -DCMAKE_CUDA_ARCHITECTURES="" . #build for detected
touch <rmm_dir>/CMakeLists.txt
ninja #should be build for detected

Before these changes the invocations of ninja would always
go back to building for all when ever ninja was invoked. The issue
is that once a CMake cache variable exists it can't be removed
via -DCMAKE_CUDA_ARCHITECTURES= and therefore becomes sticky.

To resolve the issue you can now pass -DCMAKE_CUDA_ARCHITECTURES=ALL
to consistently get all archs, and now the build-system should not
change what CUDA archs you are building for.

Consider the following:
```
cmake -DCMAKE_CUDA_ARCHITECTURES="" . #build for detected
touch <rmm_dir>/CMakeLists.txt
ninja #should be build for detected

cmake -DCMAKE_CUDA_ARCHITECTURES= . #build for all
touch <rmm_dir>/CMakeLists.txt
ninja #should be build for all

cmake -DCMAKE_CUDA_ARCHITECTURES="" . #build for detected
touch <rmm_dir>/CMakeLists.txt
ninja #should be build for detected
```

Before these changes the invocations of `ninja` would always
go back to building for all when ever `ninja` was invoked. The issue
is that once a CMake cache variable exists it can't be removed
via `-DCMAKE_CUDA_ARCHITECTURES=` and therefore becomes sticky.

To resolve the issue you can now pass `-DCMAKE_CUDA_ARCHITECTURES=ALL`
to consistently get all archs, and now the build-system should not
change what CUDA archs you are building for.
@robertmaynard robertmaynard requested a review from a team as a code owner March 12, 2021 16:13
@github-actions github-actions bot added the CMake label Mar 12, 2021
@kkraus14 kkraus14 added bug Something isn't working non-breaking Non-breaking change labels Mar 12, 2021
@kkraus14 kkraus14 added this to PR-WIP in v0.19 Release via automation Mar 12, 2021
@kkraus14
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 304c04d into rapidsai:branch-0.19 Mar 12, 2021
v0.19 Release automation moved this from PR-WIP to Done Mar 12, 2021
@trxcllnt
Copy link
Contributor

@robertmaynard is this working for you with -DCMAKE_CUDA_ARCHITECTURES=? I'm seeing this error from enable_language(CUDA), presumably because it's called before we include SetGPUArchs.cmake:

cmake-3.18/Modules/CMakeDetermineCUDACompiler.cmake:539 (message):
  Failed to find a working CUDA architecture.
Call Stack (most recent call first):
  CMakeLists.txt:100 (enable_language)

rapids-bot bot pushed a commit that referenced this pull request Mar 14, 2021
Fixes regression from #726 in auto-detecting GPU architectures when `-DCMAKE_CUDA_ARCHITECTURES=` is passed on the CLI.

Now that the cached `CMAKE_CUDA_ARCHITECTURES` isn't unset before calling `enable_language(CUDA)`, this call throws an error and configuration fails. This change ensures we call `enable_language(CUDA)` after any potential rewrites of `CMAKE_CUDA_ARCHITECTURES`.

Authors:
  - Paul Taylor (@trxcllnt)

Approvers:
  - Keith Kraus (@kkraus14)

URL: #727
@robertmaynard
Copy link
Contributor Author

@robertmaynard is this working for you with -DCMAKE_CUDA_ARCHITECTURES=? I'm seeing this error from enable_language(CUDA), presumably because it's called before we include SetGPUArchs.cmake:

cmake-3.18/Modules/CMakeDetermineCUDACompiler.cmake:539 (message):
  Failed to find a working CUDA architecture.
Call Stack (most recent call first):
  CMakeLists.txt:100 (enable_language)

Yes, executing this with a clean build directory for me. I am on a machine with a GPU.

@robertmaynard robertmaynard deleted the rmm_consistent_cuda_archs branch March 15, 2021 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CMake non-breaking Non-breaking change
Projects
No open projects
v0.19 Release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants