cmake-rdc: add support for MODULE library. #848
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
And a few cleanups (See STREQUAL usage)
sethrj
approved these changes
Jul 10, 2023
There was a problem hiding this comment.
Looks good to me, I guess it was quite a simple change after all. You said that replacing this with the standalone https://github.com/pcanal/CudaRDCExercise will happen in a separate commit?
sethrj
reviewed
Jul 10, 2023
| arg STREQUAL "EXCLUDE_FROM_ALL" OR | ||
| arg STREQUAL "STATIC" OR | ||
| arg STREQUAL "SHARED" OR | ||
| arg STREQUAL "MODULE" |
There was a problem hiding this comment.
Maybe simplify to arg MATCHES "^[A-Z_]+$"? All source files will have an extension.
There was a problem hiding this comment.
Yes, I could do that .. note that this code is literally inspired by Kitware's code in FindCUDA.mk: Kitware/CMake@b8b53db/Modules/FindCUDA.cmake#L1327
There was a problem hiding this comment.
The FindCUDA is very old but you have a point... I'm just concerned about more library arguments being added by kitware.
drbenmorgan
approved these changes
Jul 11, 2023
sethrj
requested changes
Jul 11, 2023
A separate PR as it is semantically unrelated. |
|
So is the replacement of "x" which made this harder to review 😬 |
sethrj
approved these changes
Jul 12, 2023
drbenmorgan
added a commit
to drbenmorgan/celeritas
that referenced
this pull request
Jul 19, 2023
drbenmorgan
added a commit
to drbenmorgan/celeritas
that referenced
this pull request
Jul 19, 2023
PR celeritas-project#848 taught celeritas_target_link_libraries that MODULE targets are always the final one in any link chain and hence to device link them correctly. Update text on MODULE vs SHARED targets to account for this update. Retain text on explicit linking of final target to cover case of projects that enforce use of SHARED targets for plugins.
sethrj
pushed a commit
that referenced
this pull request
Jul 26, 2023
* Document how to link loadable shared libraries with Celeritas Integration of Celeritas in ATLAS's FullSimLight required it to be used in and linked from a shared library loaded by the FullSimLight application at runtime (a "plugin"). `celeritas_target_link_libraries` on the plugin library did not result in the needed "_final" device code library being linked to it, leading to undefined symbols at load time. Traced to `celeritas_target_link_libraries` not considering a shared library as an endpoint required full device link resolution. Briefly describe this use case in documentation and provide example of how to link a "plugin" shared library to Celeritas targets. * Clarify shared/module library linking after RDC update PR #848 * Clarify shared/module library linking after RDC update PR #848 taught celeritas_target_link_libraries that MODULE targets are always the final one in any link chain and hence to device link them correctly. Update text on MODULE vs SHARED targets to account for this update. Retain text on explicit linking of final target to cover case of projects that enforce use of SHARED targets for plugins. * Address review comments on Celeritas CMake wrappers Show use of Celeritas wrappers in all places, noting decay properties allowing any combination of CUDA/CPU compiling and linking. * Clarify use cases for linking to SHARED plugin library Celeritas recommends use of celeritas_add_library to support the full RDC chain but it may not be possible to use this in projects that provide their own wrappers over the raw CMake commands. Add clarification on the use case here, retaining example use of raw CMake commands for simplest possible use case.
sethrj
added
core
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.
And a few cleanups (See STREQUAL usage)