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

[FEA] Implement a function to link a target appropriately to the CUDA runtime #427

Closed
vyasr opened this issue Jun 9, 2023 · 2 comments · Fixed by #429
Closed

[FEA] Implement a function to link a target appropriately to the CUDA runtime #427

vyasr opened this issue Jun 9, 2023 · 2 comments · Fixed by #429
Labels
? - Needs Triage Need team to review and classify feature request New feature or request

Comments

@vyasr
Copy link
Contributor

vyasr commented Jun 9, 2023

Is your feature request related to a problem? Please describe.
rapids-cmake offers rapids_cuda_init_runtime, which implicitly changes the (default) CUDA linking behavior for all subsequent targets. This implicit behavior makes it difficult for consumers to reason about the CUDA linking behavior of targets since the resulting code is order-dependent and may be overridden by settings on particular targets.

Describe the solution you'd like
We should consider implementing a new function that accepts a target and adds a linkage to the appropriate CUDA library (dynamic or static) based on a function parameter. We could implement this either via a direct target_link_libraries call under the hood, or by creating a helper target (e.g. add_library(rapids_cmake_cuda_lib...)) that encodes the linkage, similar to how we handle conda in rapids_cmake_conda_env. If we wanted to incorporate math libraries we could include those as well, but we need to ensure that those are not constrained to the same type as the CUDA runtime (since the most common long-term behavior we want to encourage is statically linking cudart but dynamically linking the math libraries). We may instead benefit more from creating a separate function to handle the math library linkage.

@vyasr vyasr added feature request New feature or request ? - Needs Triage Need team to review and classify labels Jun 9, 2023
@robertmaynard
Copy link
Contributor

We can't go with an API that maches rapids_cmake_conda_env style. The problem is that we need to set a property on the target which isn't possible via linking to a target.

I believe we will need an api that looks like:

add_library(uses_cuda SHARED <.....>)

rapids_cuda_set_runtime(uses_cuda USE_STATIC_RUNTIME ${PROJ_CUDA_STATIC_RUNTIME})

This would cause uses_cuda to initialize the target property CUDA_RUNTIME_LIBRARY to the correct value, and also link to CUDA::cudart or CUDA::cudart_static.

@robertmaynard
Copy link
Contributor

This does open the question if we also need to offer controls over the usage behavior of the linking.

So do we actually need

add_library(uses_cuda SHARED <.....>)

rapids_cuda_set_runtime(uses_cuda PUBLIC USE_STATIC_RUNTIME ${PROJ_CUDA_STATIC_RUNTIME})

@rapids-bot rapids-bot bot closed this as completed in #429 Jun 29, 2023
rapids-bot bot pushed a commit that referenced this issue Jun 29, 2023
Add `rapids_cuda_set_runtime` which is the per target version of `rapids_cuda_init_runtime`.
Fixes #427

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #429
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
? - Needs Triage Need team to review and classify feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants