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

Feat cuda version #144

Closed

Conversation

haibozhang123
Copy link

@haibozhang123 haibozhang123 commented Aug 22, 2023

support specaified CUDA Version

Issue:
In some cases, system have different version's CUDA toolkit ,so it need to specified CUDA version param(like Windows CI client-server ,and need to build history code to get stable unified products )

so it to need to specified CUDA version
If special CUDA version , bazel test need to get special CUDA version's shared library runtime file ( dll file)
effect :
if not set cuda_version or dll_runtime_system_provided param (default none or True), it will be no impact
if set CUDA version and can not find cuda_version 's CUDA toolkit, it will find CUDA in system as old logic, it will be no impact

WORKSPACE.bazel Outdated
@@ -7,7 +7,7 @@ local_repository(

load("//cuda:repositories.bzl", "register_detected_cuda_toolchains", "rules_cuda_dependencies")

rules_cuda_dependencies()
rules_cuda_dependencies("11.8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can't be hardcoded as this breaks the CI tests of different versions of CUDA

@@ -14,6 +14,5 @@ local_repository(

load("@rules_cuda//cuda:repositories.bzl", "register_detected_cuda_toolchains", "rules_cuda_dependencies")

rules_cuda_dependencies()

rules_cuda_dependencies("11.8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

target_compatible_with = ["@platforms//os:windows"],
)

cc_import(
name = "cudadevrt_lib",
interface_library = "cuda/lib/x64/cudadevrt.lib",
system_provided = 1,
#shared_library = "cuda/bin/cudart64_"+ str(cuda_package_version("cuda_cudart")) + ".dll",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this one commented out?

@@ -52,14 +55,15 @@ cc_library(
cc_import(
name = "cudart_lib",
interface_library = "cuda/lib/x64/cudart.lib",
system_provided = 1,
shared_library = "cuda/bin/cudart64_"+ str(cuda_package_version("cuda_cudart")) + ".dll",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it might be x64 specific from the name? Do you need to also provide the 32 bit variants of these libraries or are they always only 64 bit? I'm not so familiar with cuda libraries on windows.
If its x64 only then the target_compatible_with option will also need updating.

@@ -232,4 +311,5 @@ def rules_cuda_dependencies(toolkit_path = None):
],
)

local_cuda(name = "local_cuda", toolkit_path = toolkit_path)
local_cuda(name = "local_cuda", cuda_version = cuda_version, toolkit_path = toolkit_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's similar calls to local_cuda that need updating in the bzlmod module extension in cuda/extensions.bzl. You'll need to expose this same cuda_version option in the modules tag_class.

@cloudhan
Copy link
Collaborator

Why is this needed?

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

Successfully merging this pull request may close these issues.

3 participants