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.

Sign up for GitHub

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

Jump to bottom

replace accesses to cc_toolchain.compiler_executable with calls to cc_toolchain.get_tool_for_action() #274

Merged
merged 12 commits into from
Sep 11, 2024
Merged

replace accesses to cc_toolchain.compiler_executable with calls to cc_toolchain.get_tool_for_action() #274

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
aa4802c
replace accesses to cc_toolchain.compiler_executable with calls to cc…
hypdeb Sep 8, 2024
1a3a6bf
get_tool_for_action is in cc_common
hypdeb Sep 8, 2024
6baef98
passing feature configuration
hypdeb Sep 9, 2024
33ad389
missing load
hypdeb Sep 9, 2024
35d2090
usage of configure_features
hypdeb Sep 9, 2024
d297ef0
correct call to configure_features
hypdeb Sep 9, 2024
6d31409
experimenting
hypdeb Sep 9, 2024
b50e8ad
added required configuration fragment
hypdeb Sep 9, 2024
4745c07
revert unnecessary changes
hypdeb Sep 10, 2024
f16c4bd
revert unnecessary changes
hypdeb Sep 10, 2024
14bb720
Merge branch 'main' into use_get_tool_for_action
hypdeb Sep 11, 2024
eb5d156
remove changes incompatible with older versions of Bazel
hypdeb Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion cuda/private/actions/compile.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", CC_ACTION_NAMES = "ACTION_NAMES")
load("//cuda/private:action_names.bzl", "ACTION_NAMES")
load("//cuda/private:cuda_helper.bzl", "cuda_helper")
load("//cuda/private:rules/common.bzl", "ALLOW_CUDA_SRCS")
Expand Down Expand Up @@ -33,7 +34,13 @@ def compile(
An compiled object `File`.
"""
actions = ctx.actions
host_compiler = cc_toolchain.compiler_executable
cc_feature_configuration = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
host_compiler = cc_common.get_tool_for_action(feature_configuration = cc_feature_configuration, action_name = CC_ACTION_NAMES.cpp_compile)
cuda_compiler = cuda_toolchain.compiler_executable

cuda_feature_config = cuda_helper.configure_features(ctx, cuda_toolchain, requested_features = [ACTION_NAMES.cuda_compile])
Expand Down
9 changes: 8 additions & 1 deletion cuda/private/actions/dlink.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
""

load("@bazel_tools//tools/build_defs/cc:action_names.bzl", CC_ACTION_NAMES = "ACTION_NAMES")
load("//cuda/private:action_names.bzl", "ACTION_NAMES")
load("//cuda/private:actions/compile.bzl", "compile")
load("//cuda/private:cuda_helper.bzl", "cuda_helper")
Expand Down Expand Up @@ -55,7 +56,13 @@ def _compiler_device_link(
fail("device link is only meaningful on building relocatable device code")

actions = ctx.actions
host_compiler = cc_toolchain.compiler_executable
cc_feature_configuration = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
host_compiler = cc_common.get_tool_for_action(feature_configuration = cc_feature_configuration, action_name = CC_ACTION_NAMES.cpp_compile)
cuda_compiler = cuda_toolchain.compiler_executable

artifact_category_name = cuda_helper.get_artifact_category_from_action(ACTION_NAMES.device_link, pic, rdc)
Expand Down
6 changes: 3 additions & 3 deletions cuda/private/rules/cuda_library.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you forgot to revert these changes

load("//cuda/private:actions/compile.bzl", "compile")
load("//cuda/private:actions/dlink.bzl", "device_link")
load("//cuda/private:cuda_helper.bzl", "cuda_helper")
load("//cuda/private:providers.bzl", "CudaInfo")
load("//cuda/private:rules/common.bzl", "ALLOW_CUDA_HDRS", "ALLOW_CUDA_SRCS")
load("//cuda/private:toolchain.bzl", "find_cuda_toolchain", "use_cpp_toolchain", "use_cuda_toolchain")
load("//cuda/private:toolchain.bzl", "find_cuda_toolchain", "use_cuda_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here.


def _cuda_library_impl(ctx):
"""cuda_library is a rule that perform device link.
Expand Down Expand Up @@ -175,6 +175,6 @@ cuda_library = rule(
"_default_cuda_archs": attr.label(default = "//cuda:archs"),
},
fragments = ["cpp"],
toolchains = use_cpp_toolchain() + use_cuda_toolchain(),
toolchains = use_cpp_toolchain(mandatory = True) + use_cuda_toolchain(),
provides = [DefaultInfo, OutputGroupInfo, CcInfo, CudaInfo],
)
6 changes: 3 additions & 3 deletions cuda/private/rules/cuda_objects.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I just now realized what you meant with your first review comment. Just to confirm: we need to use the use_cpp_toolchain defined in rules_cuda because we support versions of Bazel where it is not defined in bazel_tools correct?

There's no way I suppose to define this one conditionally on the Bazel version? Not that I necessarily think it would be better, mostly out of curiosity.

load("//cuda/private:actions/compile.bzl", "compile")
load("//cuda/private:cuda_helper.bzl", "cuda_helper")
load("//cuda/private:providers.bzl", "CudaInfo")
load("//cuda/private:rules/common.bzl", "ALLOW_CUDA_HDRS", "ALLOW_CUDA_SRCS")
load("//cuda/private:toolchain.bzl", "find_cuda_toolchain", "use_cpp_toolchain", "use_cuda_toolchain")
load("//cuda/private:toolchain.bzl", "find_cuda_toolchain", "use_cuda_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here.


def _cuda_objects_impl(ctx):
attr = ctx.attr
Expand Down Expand Up @@ -110,6 +110,6 @@ code and device link time optimization source files.""",
"_default_cuda_archs": attr.label(default = "//cuda:archs"),
},
fragments = ["cpp"],
toolchains = use_cpp_toolchain() + use_cuda_toolchain(),
toolchains = use_cpp_toolchain(mandatory = True) + use_cuda_toolchain(),
provides = [DefaultInfo, OutputGroupInfo, CcInfo, CudaInfo],
)
16 changes: 12 additions & 4 deletions cuda/private/toolchain_configs/clang.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", CC_ACTION_NAMES = "ACTION_NAMES")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here.

load("//cuda/private:action_names.bzl", "ACTION_NAMES")
load("//cuda/private:artifact_categories.bzl", "ARTIFACT_CATEGORIES")
load("//cuda/private:providers.bzl", "CudaToolchainConfigInfo", "CudaToolkitInfo")
load("//cuda/private:toolchain.bzl", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

load(
"//cuda/private:toolchain_config_lib.bzl",
"action_config",
Expand Down Expand Up @@ -52,6 +52,13 @@ def _impl(ctx):
]

cc_toolchain = find_cpp_toolchain(ctx)
cc_feature_configuration = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
host_compiler = cc_common.get_tool_for_action(feature_configuration = cc_feature_configuration, action_name = CC_ACTION_NAMES.cpp_compile)

clang_compile_env_feature = feature(
name = "clang_compile_env",
Expand All @@ -64,7 +71,7 @@ def _impl(ctx):
],
env_entries = [
env_entry("INCLUDE", ";".join(cc_toolchain.built_in_include_directories)),
env_entry("PATH", paths.dirname(cc_toolchain.compiler_executable) + ";C:/Windows/system32"),
env_entry("PATH", paths.dirname(host_compiler) + ";C:/Windows/system32"),
],
),
],
Expand Down Expand Up @@ -508,5 +515,6 @@ cuda_toolchain_config = rule(
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), # legacy behaviour
},
provides = [CudaToolchainConfigInfo],
toolchains = use_cpp_toolchain(),
toolchains = use_cpp_toolchain(mandatory = True),
fragments = ["cpp"],
)
18 changes: 13 additions & 5 deletions cuda/private/toolchain_configs/nvcc.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", CC_ACTION_NAMES = "ACTION_NAMES")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

load("//cuda/private:action_names.bzl", "ACTION_NAMES")
load("//cuda/private:artifact_categories.bzl", "ARTIFACT_CATEGORIES")
load("//cuda/private:providers.bzl", "CudaToolchainConfigInfo", "CudaToolkitInfo")
load("//cuda/private:toolchain.bzl", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

load(
"//cuda/private:toolchain_config_lib.bzl",
"action_config",
Expand Down Expand Up @@ -42,13 +42,20 @@ def _impl(ctx):
]

cc_toolchain = find_cpp_toolchain(ctx)
cc_feature_configuration = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
host_compiler = cc_common.get_tool_for_action(feature_configuration = cc_feature_configuration, action_name = CC_ACTION_NAMES.cpp_compile)

nvcc_compile_env_feature = feature(
name = "nvcc_compile_env",
env_sets = [
env_set(
actions = [ACTION_NAMES.cuda_compile],
env_entries = [env_entry("PATH", paths.dirname(cc_toolchain.compiler_executable))],
env_entries = [env_entry("PATH", paths.dirname(host_compiler))],
),
],
)
Expand All @@ -58,7 +65,7 @@ def _impl(ctx):
env_sets = [
env_set(
actions = [ACTION_NAMES.device_link],
env_entries = [env_entry("PATH", paths.dirname(cc_toolchain.compiler_executable))],
env_entries = [env_entry("PATH", paths.dirname(host_compiler))],
),
],
)
Expand Down Expand Up @@ -518,5 +525,6 @@ cuda_toolchain_config = rule(
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), # legacy behaviour
},
provides = [CudaToolchainConfigInfo],
toolchains = use_cpp_toolchain(),
toolchains = use_cpp_toolchain(mandatory = True),
fragments = ["cpp"],
)
18 changes: 12 additions & 6 deletions cuda/private/toolchain_configs/nvcc_msvc.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load("@bazel_tools//tools/build_defs/cc:action_names.bzl", CC_ACTION_NAMES = "ACTION_NAMES")
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

load("//cuda/private:action_names.bzl", "ACTION_NAMES")
load("//cuda/private:artifact_categories.bzl", "ARTIFACT_CATEGORIES")
load("//cuda/private:providers.bzl", "CudaToolchainConfigInfo", "CudaToolkitInfo")
load("//cuda/private:toolchain.bzl", "use_cpp_toolchain")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

load(
"//cuda/private:toolchain_config_lib.bzl",
"action_config",
Expand Down Expand Up @@ -43,6 +43,13 @@ def _impl(ctx):
]

cc_toolchain = find_cpp_toolchain(ctx)
cc_feature_configuration = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
host_compiler = cc_common.get_tool_for_action(feature_configuration = cc_feature_configuration, action_name = CC_ACTION_NAMES.cpp_compile)

nvcc_compile_env_feature = feature(
name = "nvcc_compile_env",
Expand All @@ -54,7 +61,7 @@ def _impl(ctx):
],
env_entries = [
env_entry("INCLUDE", ";".join(cc_toolchain.built_in_include_directories)),
env_entry("PATH", paths.dirname(cc_toolchain.compiler_executable) + ";C:/Windows/system32"),
env_entry("PATH", paths.dirname(host_compiler) + ";C:/Windows/system32"),
env_entry("TEMP", ctx.attr.msvc_env_tmp),
env_entry("TMP", ctx.attr.msvc_env_tmp),
],
Expand Down Expand Up @@ -430,8 +437,6 @@ def _impl(ctx):
],
)

static_link_msvcrt_feature = feature(name = "static_link_msvcrt")

static_link_msvcrt_debug_feature = feature(
name = "static_link_msvcrt_debug",
flag_sets = [
Expand Down Expand Up @@ -608,5 +613,6 @@ cuda_toolchain_config = rule(
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), # legacy behaviour
},
provides = [CudaToolchainConfigInfo],
toolchains = use_cpp_toolchain(),
toolchains = use_cpp_toolchain(mandatory = True),
fragments = ["cpp"],
)