This repository was archived by the owner on Mar 20, 2023. It is now read-only.
Modernise CMake/CUDA and fix link issues #609
Merged
Conversation
This file contains hidden or 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
Make sure device code linking only happens once, rather than linking explicit CUDA code earlier and linking OpenACC device code later. CUDA has first class language support in all recent CMake versions, so remove find_package(CUDA) and retire the deprecated `cuda_add_library` function. Retire the CORENRN_GPU_CUDA_COMPUTE_CAPABILITY CMake variable and use the standard CMAKE_CUDA_ARCHTECTURES one instead.
This avoids __CUDACC__ and CUDA features being enabled when compiling .cpp files, which breaks assumptions (but might be fine in the long run). Also prefix some preprocessor macro names with CORENEURON_.
Collaborator
|
Logfiles from GitLab pipeline #13365 (:no_entry:) have been uploaded here! Status and direct links:
|
505b7cd to
8e6ff6b
Compare
Collaborator
|
Logfiles from GitLab pipeline #13373 (:no_entry:) have been uploaded here! Status and direct links:
|
Collaborator
|
Logfiles from GitLab pipeline #13388 (:no_entry:) have been uploaded here! Status and direct links:
|
Collaborator
|
Logfiles from GitLab pipeline #13387 (:no_entry:) have been uploaded here! Status and direct links:
|
Drop other -D argument that is now injected via the `localrc` file of BB5's PGI/NVHPC compiler installations.
Collaborator
|
Logfiles from GitLab pipeline #13408 (:white_check_mark:) have been uploaded here! Status and direct links:
|
Collaborator
|
Logfiles from GitLab pipeline #13416 (:white_check_mark:) have been uploaded here! Status and direct links:
|
Contributor
Author
|
We should merge BlueBrain/spack#1249 immediately before merging this. |
Contributor
|
Please retest |
alexsavulescu
approved these changes
Aug 13, 2021
This was referenced Aug 18, 2021
pramodk
pushed a commit
to neuronsimulator/nrn
that referenced
this pull request
Nov 2, 2022
* clang-format-12 support in hpc-coding-conventions. * Fix CUDA/GPU linking and avoid deprecated CMake. Make sure device code linking only happens once, rather than linking explicit CUDA code earlier and linking OpenACC device code later. CUDA has first class language support in all recent CMake versions, so remove find_package(CUDA) and retire the deprecated `cuda_add_library` function. Retire the CORENRN_GPU_CUDA_COMPUTE_CAPABILITY CMake variable and use the standard CMAKE_CUDA_ARCHTECTURES one instead. * Only pass -cuda when linking. This avoids __CUDACC__ and CUDA features being enabled when compiling .cpp files, which breaks assumptions (but might be fine in the long run). Also prefix some preprocessor macro names with CORENEURON_. * Consistent libcudart linkage. * Fix silly error for -DNDEBUG. * Update README to suggest CMAKE_CUDA_COMPILER=nvcc. * CMake minimum v3.15 for GPU builds. * Tweaks for older CMake versions. * Set CMAKE_CUDA_COMPILER=nvcc. Drop other -D argument that is now injected via the `localrc` file of BB5's PGI/NVHPC compiler installations. CoreNEURON Repo SHA: BlueBrain/CoreNeuron@170a0bb
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
This changeset changes the CMake configuration of GPU builds to:
find_package(CUDA ...),cuda_add_library(...), etc.CORENRN_GPU_CUDA_COMPUTE_CAPABILITYoption; we now use the standardCMAKE_CUDA_ARCHITECTURESvariableclang-format-12.These changes force compilation to proceed slightly differently to a standard mixed CUDA/C++ project. Instead of allowing CMake to generate an explicit device linker step, i.e.
before compiling OpenACC/GPU-enabled C++ code (which emits additional device code):
we instead let
nvc++do the device code linking itself, i.e. something more likewhich seems to give correct results. This is a bit tortuous in CMake presumably because the same pattern would fail with a GPU-unaware C++ compiler, e.g. GCC or Clang. The hypothesis is that the old way of doing things fell foul of
from the CUDA documentation, while the new way ensures there is a single device link step including both the code generated from
.cufiles and that from OpenACC regions.We now also prefer to dynamically link the CUDA runtime,
libcudart.so.nvc++ -cudaseems to prefer this and only allows it to be steered by the-static-nvidiaoption, which would also statically link the OpenACC runtime (which has always been dynamically linked). SettingCMAKE_CUDA_RUNTIME_LIBRARY=Sharedstops CMake from emitting-lcudart_static, which causes segfaults at teardown in combination with-cuda's dynamic linking.cc: @kotsaloscv
Closes #520. Closes #607.
How to test this?
Follow instructions in #607 to test the link/global state issue.
Test System
Use certain branches for the SimulationStack CI
CI_BRANCHES:NEURON_BRANCH=master,