[REVIEW] Use CMAKE_CUDA_ARCHITECTURES #7391
Merged
Conversation
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
github-actions
bot
added
CMake
kkraus14
reviewed
Feb 16, 2021
I think that move is reasonable. Maybe after / part of #7107? It would allow better compilation flags control. If we ever needed targets built for a subset of archs we could. |
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7391 +/- ##
==============================================
Coverage ? 82.19%
==============================================
Files ? 100
Lines ? 16968
Branches ? 0
==============================================
Hits ? 13947
Misses ? 3021
Partials ? 0 Continue to review full report at Codecov.
|
harrism
added
non-breaking
harrism
changed the title
harrism
reviewed
Feb 16, 2021
harrism
reviewed
Feb 16, 2021
harrism
approved these changes
Feb 16, 2021
kkraus14
approved these changes
Feb 17, 2021
|
rerun tests |
robertmaynard
changed the title
robertmaynard
force-pushed
the
correct_CMP0104_warnings
branch
from
473169d
to
db5c7d3
Compare
robertmaynard
force-pushed
the
correct_CMP0104_warnings
branch
from
db5c7d3
to
d319394
Compare
robertmaynard
changed the title
kkraus14
approved these changes
Feb 19, 2021
kkraus14
added
breaking
|
Pinged some folks offline regarding rapids-compose and devops scripts to make sure no one is using |
|
@gpucibot merge |
rapids-bot bot
pushed a commit
to rapidsai/rmm
that referenced
this pull request
Feb 24, 2021
Based on #706 as they both modify `cmake/Modules/SetGPUArchs.cmake` This brings rmm's handling of `CMAKE_CUDA_ARCHITECTURES` to match that is proposed for cudf in rapidsai/cudf#7391 Authors: - Robert Maynard (@robertmaynard) Approvers: - Mark Harris (@harrism) - Keith Kraus (@kkraus14) URL: #709
rapids-bot bot
pushed a commit
that referenced
this pull request
Feb 24, 2021
This eliminates the `Policy CMP0104 is not set` warning during the JNI build by using `CMAKE_CUDA_ARCHITECTURES` to specify the targeted CUDA architectures during the build. This eliminates a lot of code replicated from the cpp build and reuses the `ConfigureCUDA` module added in #7391. The architectures being used by the build are visible in the `mvn` output, e.g.: ``` [exec] -- CUDF: Building CUDF for GPU architectures: 60-real;70-real;75 ``` This also configures the CPU compiler to use the same flags as the cpp build which required fixing a number of warnings (e.g.: sign mismatch comparisons, unused variables, etc.) in the JNI code since warnings are errors in the build. Authors: - Jason Lowe (@jlowe) Approvers: - Robert (Bobby) Evans (@revans2) - Alessandro Bellina (@abellina) - MithunR (@mythrocks) URL: #7425
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When using CMake >= 3.18 you now don't get
CMP0104warnings for each target that use CUDA.To correct this issue I have cherry-picked a collection of changes from #7107 which updated cudf to use
CMAKE_CUDA_ARCHITECTURES.When using CMake < 3.18 cudf will transform
CMAKE_CUDA_ARCHITECTURESinto the correct flags and store it intoCMAKE_CUDA_FLAGS.When I was porting this over from !7107 I noticed that when cudf is built for multi arch ( sm60, sm70, sm80, ... ) it only compiles compute for the newest arch. This behavior is preserved, and !7107 will need to be updated with this change.