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

Why @rules_cuda//cuda:runtime flag rather than respecting --dynamic_mode #119

Open
garymm opened this issue Jul 3, 2023 · 5 comments
Open

Comments

@garymm
Copy link
Contributor

garymm commented Jul 3, 2023

Bazel already has a flag (--dynamic_mode) to allow controlling of static vs dynamic linking. I think it's feasible to apply this to the cuda runtime. Why not use this flag? It's surprising that there's a separate flag and that the bazel flag is not respected.

If you're OK with switching to use the built-in --dynamic_mode I'd be happy to open a PR to implement the change.
Thanks!

@cloudhan
Copy link
Collaborator

cloudhan commented Jul 4, 2023

This is from

rules_cuda/cuda/BUILD

Lines 91 to 103 in 152bbbc

# Command line flag to specify the CUDA runtime. Use this target as CUDA
# runtime dependency.
#
# This target is implicitly added as a dependency to cuda_library() targets.
#
# Example usage: --@rules_cuda//cuda:cuda_runtime=@local_cuda//:cuda_runtime
label_flag(
name = "cuda_runtime",
build_setting_default = if_local_cuda(
"@local_cuda//:cuda_runtime_static",
":empty",
),
)
, this repo originally is a subtree of tf_runtime and then a full rewrite is carried out starts (with an independent source tree root).

@garymm
Copy link
Contributor Author

garymm commented Jul 5, 2023

@cloudhan are you OK with me removing the flag and just relying on --dynamic_mode? Or if not, then having the default be to respect --dynamic_mode?

@cloudhan
Copy link
Collaborator

cloudhan commented Jul 5, 2023

I don't know the reason behind it actually, so let's keep it as is for now. Maybe @chsigg can explain it a bit?

@chsigg
Copy link

chsigg commented Jul 6, 2023

The flag is also introduced to inject a different CUDA runtime, e.g. one that dynamically loads the real CUDA runtime and forwards the calls. That can be independent though of (by default) dynamically switching between linking against static or dynamic runtime. I don't know how to dynamically switch between the two based on --dynamic_mode. You could probably use cc_import(), but you might need to wrap that in a cc_library() still for the existing linkopts arguments. This might be worthwhile doing, although NVIDIA strongly advises against linking dynamic CUDA runtime.

@garymm
Copy link
Contributor Author

garymm commented Jul 6, 2023

If you assign this to me I can work on making it respect --dynamic_mode by default, and we can leave the flag to override.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants