From 8681437cdcbc8687f008c601b2b910f11710d19f Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 6 Mar 2023 01:09:01 -0800 Subject: [PATCH] Add automatic exec groups to cc_common.link PiperOrigin-RevId: 514332514 Change-Id: I5ea3b2fa70a858c8d206a36ede1eec02fb344bf2 --- .../bazel/rules/cpp/BazelCppSemantics.java | 7 + .../google/devtools/build/lib/rules/cpp/BUILD | 4 +- .../lib/rules/cpp/CppLinkActionBuilder.java | 17 +- .../build/lib/rules/cpp/CppRuleClasses.java | 9 +- .../build/lib/rules/cpp/CppSemantics.java | 4 + .../lib/analysis/AutoExecGroupsTest.java | 281 ++++++++++++++++++ .../build/lib/rules/cpp/MockCppSemantics.java | 7 + .../build/lib/testutil/TestConstants.java | 3 + 8 files changed, 326 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java index 363929e1d2078c..593e839b4c62d8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java @@ -75,6 +75,13 @@ private BazelCppSemantics(Language language) { this.language = language; } + private static final String CPP_TOOLCHAIN_TYPE = "@bazel_tools//tools/cpp:toolchain_type"; + + @Override + public String getCppToolchainType() { + return CPP_TOOLCHAIN_TYPE; + } + @Override public Language language() { return language; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD index 630ff587ff05b2..ba29fb54fe143b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD @@ -71,10 +71,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration", "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/args", - "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_api_provider", - "//src/main/java/com/google/devtools/build/lib/analysis:statically_linked_marker_provider", "//src/main/java/com/google/devtools/build/lib/analysis:template_variable_info", - "//src/main/java/com/google/devtools/build/lib/analysis:test/instrumented_files_info", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider", "//src/main/java/com/google/devtools/build/lib/analysis:workspace_status_action", @@ -91,6 +88,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_resolver", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages:exec_group", "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/rules:alias", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index e2e61c0517168f..e5fb5b2863f788 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -1159,8 +1159,21 @@ public static ImmutableMap mapLinkstampsToOutputs( } protected ActionOwner getOwner() { - ActionOwner execGroupOwner = actionConstructionContext.getActionOwner(CPP_LINK_EXEC_GROUP); - return execGroupOwner == null ? actionConstructionContext.getActionOwner() : execGroupOwner; + ActionOwner cppLinkExecGroupOwner = + actionConstructionContext.getActionOwner(CPP_LINK_EXEC_GROUP); + if (cppLinkExecGroupOwner != null) { + return cppLinkExecGroupOwner; + } + + if (((RuleContext) actionConstructionContext).useAutoExecGroups()) { + ActionOwner autoExecGroupOwner = + actionConstructionContext.getActionOwner(cppSemantics.getCppToolchainType()); + return autoExecGroupOwner == null + ? actionConstructionContext.getActionOwner() + : autoExecGroupOwner; + } + + return actionConstructionContext.getActionOwner(); } /** Sets the mnemonic for the link action. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index c21de5d26a2096..f586c0f3826c67 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -33,6 +33,7 @@ import static com.google.devtools.build.lib.rules.cpp.CppFileTypes.VERSIONED_SHARED_LIBRARY; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; @@ -41,6 +42,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault.Resolver; +import com.google.devtools.build.lib.packages.ExecGroup; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -554,7 +556,12 @@ public Metadata getMetadata() { public static final class CcLinkingRule implements RuleDefinition { @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { - return builder.addExecGroup(CPP_LINK_EXEC_GROUP).build(); + return builder + .addExecGroups( + ImmutableMap.of( + CPP_LINK_EXEC_GROUP, + ExecGroup.builder().addToolchainType(ccToolchainTypeRequirement(env)).build())) + .build(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java index 9aa55217efda1e..d2f3bf0b3b7364 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java @@ -33,6 +33,10 @@ /** Pluggable C++ compilation semantics. */ public interface CppSemantics extends StarlarkValue { + + /** Returns cpp toolchain type. */ + String getCppToolchainType(); + /** What language to treat the headers. */ Language language(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AutoExecGroupsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AutoExecGroupsTest.java index 9ed5d52d34a46b..d8dc6f522fd259 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AutoExecGroupsTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AutoExecGroupsTest.java @@ -16,6 +16,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.CPP_LINK_EXEC_GROUP; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -23,13 +24,19 @@ import com.google.common.collect.ObjectArrays; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.analysis.actions.LazyWritePathsFileAction; +import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement; import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo; +import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.ExecGroup; +import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig; +import com.google.devtools.build.lib.rules.cpp.CppCompileAction; +import com.google.devtools.build.lib.rules.cpp.CppLinkAction; +import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; import com.google.devtools.build.lib.rules.java.JavaGenJarsProvider; import com.google.devtools.build.lib.rules.java.JavaInfo; import com.google.devtools.build.lib.testutil.TestConstants; @@ -1390,4 +1397,278 @@ public void javaCommonStampJar_automaticExecGroupsEnabled_actionExecutesOnFirstP assertThat(actions.get(0).getOwner().getExecutionPlatform().label()) .isEqualTo(Label.parseCanonical("//platforms:platform_1")); } + + @Test + public void ccCommonLink_fileWriteActionExecutesOnFirstPlatform() throws Exception { + scratch.file( + "test/defs.bzl", + "def _use_cpp_toolchain():", + " return [", + " config_common.toolchain_type('" + + TestConstants.CPP_TOOLCHAIN_TYPE + + "', mandatory = False),", + " ]", + "def _impl(ctx):", + " cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]", + " feature_configuration = cc_common.configure_features(", + " ctx = ctx,", + " cc_toolchain = cc_toolchain,", + " requested_features = ctx.features,", + " unsupported_features = ctx.disabled_features,", + " )", + " linking_outputs = cc_common.link(", + " name = ctx.label.name,", + " actions = ctx.actions,", + " feature_configuration = feature_configuration,", + " cc_toolchain = cc_toolchain,", + " )", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " attrs = { '_cc_toolchain': attr.label(default=Label('//test:alias')) },", + " toolchains = ['//rule:toolchain_type_2'] + _use_cpp_toolchain(),", + " fragments = ['cpp']", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'custom_rule')", + "cc_toolchain_alias(name='alias')", + "custom_rule(name = 'custom_rule_name')"); + useConfiguration("--incompatible_auto_exec_groups"); + + ImmutableList actions = + getActions("//test:custom_rule_name", ParameterFileWriteAction.class); + + assertThat(actions).hasSize(1); + assertThat(actions.get(0).getOwner().getExecutionPlatform().label()) + .isEqualTo(Label.parseCanonical("//platforms:platform_1")); + } + + @Test + public void ccCommonLink_cppLinkExecGroupNotDefined_cppLinkActionExecutesOnFirstPlatform() + throws Exception { + scratch.file( + "test/defs.bzl", + "def _use_cpp_toolchain():", + " return [", + " config_common.toolchain_type('" + + TestConstants.CPP_TOOLCHAIN_TYPE + + "', mandatory = False),", + " ]", + "def _impl(ctx):", + " cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]", + " feature_configuration = cc_common.configure_features(", + " ctx = ctx,", + " cc_toolchain = cc_toolchain,", + " requested_features = ctx.features,", + " unsupported_features = ctx.disabled_features,", + " )", + " linking_outputs = cc_common.link(", + " name = ctx.label.name,", + " actions = ctx.actions,", + " feature_configuration = feature_configuration,", + " cc_toolchain = cc_toolchain,", + " )", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " attrs = { '_cc_toolchain': attr.label(default=Label('//test:alias')) },", + " exec_groups = { ", + " '" + CPP_LINK_EXEC_GROUP + "': exec_group(toolchains = _use_cpp_toolchain()),", + " },", + " toolchains = ['//rule:toolchain_type_2'] + _use_cpp_toolchain(),", + " fragments = ['cpp']", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'custom_rule')", + "cc_toolchain_alias(name='alias')", + "custom_rule(name = 'custom_rule_name')"); + useConfiguration("--incompatible_auto_exec_groups"); + + ImmutableList actions = getActions("//test:custom_rule_name", CppLinkAction.class); + + assertThat(actions).hasSize(1); + assertThat(actions.get(0).getMnemonic()).isEqualTo("CppLink"); + assertThat(actions.get(0).getOwner().getExecutionPlatform().label()) + .isEqualTo(Label.parseCanonical("//platforms:platform_1")); + } + + @Test + public void ccCommonLink_cppLinkExecGroupDefined_cppLinkActionExecutesOnFirstPlatform() + throws Exception { + scratch.file( + "test/defs.bzl", + "def _use_cpp_toolchain():", + " return [", + " config_common.toolchain_type('" + + TestConstants.CPP_TOOLCHAIN_TYPE + + "', mandatory = False),", + " ]", + "def _impl(ctx):", + " cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]", + " feature_configuration = cc_common.configure_features(", + " ctx = ctx,", + " cc_toolchain = cc_toolchain,", + " requested_features = ctx.features,", + " unsupported_features = ctx.disabled_features,", + " )", + " linking_outputs = cc_common.link(", + " name = ctx.label.name,", + " actions = ctx.actions,", + " feature_configuration = feature_configuration,", + " cc_toolchain = cc_toolchain,", + " )", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " attrs = { '_cc_toolchain': attr.label(default=Label('//test:alias')) },", + " exec_groups = { ", + " '" + CPP_LINK_EXEC_GROUP + "': exec_group(toolchains = _use_cpp_toolchain()),", + " },", + " toolchains = ['//rule:toolchain_type_2'] + _use_cpp_toolchain(),", + " fragments = ['cpp']", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'custom_rule')", + "cc_toolchain_alias(name='alias')", + "custom_rule(name = 'custom_rule_name')"); + useConfiguration("--incompatible_auto_exec_groups"); + + ImmutableList actions = getActions("//test:custom_rule_name", CppLinkAction.class); + + assertThat(actions).hasSize(1); + assertThat(actions.get(0).getMnemonic()).isEqualTo("CppLink"); + assertThat(actions.get(0).getOwner().getExecutionPlatform().label()) + .isEqualTo(Label.parseCanonical("//platforms:platform_1")); + } + + @Test + public void ccCommonLink_cppLTOActionExecutesOnFirstPlatform() throws Exception { + scratch.file( + "test/defs.bzl", + "def _use_cpp_toolchain():", + " return [", + " config_common.toolchain_type('" + + TestConstants.CPP_TOOLCHAIN_TYPE + + "', mandatory = False),", + " ]", + "def _impl(ctx):", + " cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]", + " feature_configuration = cc_common.configure_features(", + " ctx = ctx,", + " cc_toolchain = cc_toolchain,", + " requested_features = ctx.features,", + " unsupported_features = ctx.disabled_features,", + " )", + " linking_outputs = cc_common.link(", + " name = ctx.label.name,", + " actions = ctx.actions,", + " feature_configuration = feature_configuration,", + " cc_toolchain = cc_toolchain,", + " linking_contexts = [dep[CcInfo].linking_context for dep in ctx.attr.deps if" + + " CcInfo in dep]", + " )", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'deps': attr.label_list(),", + " 'srcs': attr.label_list(allow_files = ['.cc']),", + " '_cc_toolchain': attr.label(default=Label('//test:alias')),", + " },", + " toolchains = ['//rule:toolchain_type_2'] + _use_cpp_toolchain(),", + " fragments = ['cpp']", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'custom_rule')", + "cc_toolchain_alias(name='alias')", + "cc_library(", + " name = 'dep',", + " srcs = ['dep.cc'],", + ")", + "custom_rule(", + " name = 'custom_rule_name',", + " srcs = ['custom.cc'],", + " deps = ['dep'],", + ")"); + useConfiguration("--incompatible_auto_exec_groups", "--features=thin_lto"); + AnalysisMock.get() + .ccSupport() + .setupCcToolchainConfig( + mockToolsConfig, + CcToolchainConfig.builder() + .withFeatures(CppRuleClasses.THIN_LTO, CppRuleClasses.SUPPORTS_START_END_LIB)); + + ImmutableList actions = getActions("//test:custom_rule_name", CppLinkAction.class); + ImmutableList cppLTOActions = + actions.stream() + .filter(action -> action.getMnemonic().equals("CppLTOIndexing")) + .collect(toImmutableList()); + + assertThat(cppLTOActions).hasSize(1); + assertThat(cppLTOActions.get(0).getOwner().getExecutionPlatform().label()) + .isEqualTo(Label.parseCanonical("//platforms:platform_1")); + } + + @Test + public void ccCommonLink_linkstampCompileActionExecutesOnFirstPlatform() throws Exception { + scratch.file( + "bazel_internal/test_rules/cc/defs.bzl", + "def _use_cpp_toolchain():", + " return [", + " config_common.toolchain_type('" + + TestConstants.CPP_TOOLCHAIN_TYPE + + "', mandatory = False),", + " ]", + "def _impl(ctx):", + " cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]", + " feature_configuration = cc_common.configure_features(", + " ctx = ctx,", + " cc_toolchain = cc_toolchain,", + " requested_features = ctx.features,", + " unsupported_features = ctx.disabled_features,", + " )", + " linking_outputs = cc_common.link(", + " name = ctx.label.name,", + " actions = ctx.actions,", + " feature_configuration = feature_configuration,", + " cc_toolchain = cc_toolchain,", + " linking_contexts = [dep[CcInfo].linking_context for dep in ctx.attr.deps if" + + " CcInfo in dep]", + " )", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " attrs = {", + " 'deps': attr.label_list(),", + " '_cc_toolchain': attr.label(default=Label('//bazel_internal/test_rules/cc:alias')),", + " },", + " toolchains = ['//rule:toolchain_type_2'] + _use_cpp_toolchain(),", + " fragments = ['cpp']", + ")"); + scratch.file( + "bazel_internal/test_rules/cc/BUILD", + "load('//bazel_internal/test_rules/cc:defs.bzl', 'custom_rule')", + "cc_toolchain_alias(name='alias')", + "cc_library(", + " name = 'dep',", + " linkstamp = 'stamp.cc',", + ")", + "custom_rule(", + " name = 'custom_rule_name',", + " deps = ['dep'],", + ")"); + useConfiguration("--incompatible_auto_exec_groups"); + + ImmutableList cppCompileActions = + getActions("//bazel_internal/test_rules/cc:custom_rule_name", CppCompileAction.class); + + assertThat(cppCompileActions).hasSize(1); + assertThat(cppCompileActions.get(0).getMnemonic()).isEqualTo("CppLinkstampCompile"); + assertThat(cppCompileActions.get(0).getOwner().getExecutionPlatform().label()) + .isEqualTo(Label.parseCanonical("//platforms:platform_1")); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java index 8190bcdfcbf96c..0e00c45c6b7edf 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java @@ -45,6 +45,13 @@ public Language language() { return Language.CPP; } + private static final String CPP_TOOLCHAIN_TYPE = "@bazel_tools//tools/cpp:toolchain_type"; + + @Override + public String getCppToolchainType() { + return CPP_TOOLCHAIN_TYPE; + } + @Override public void finalizeCompileActionBuilder( BuildConfigurationValue configuration, diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java index a4967e380265b8..aa1bf27355b2f6 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java @@ -167,6 +167,9 @@ private TestConstants() { /** The java toolchain type. */ public static final String JAVA_TOOLCHAIN_TYPE = "@bazel_tools//tools/jdk:toolchain_type"; + /** The cpp toolchain type. */ + public static final String CPP_TOOLCHAIN_TYPE = "@bazel_tools//tools/cpp:toolchain_type"; + /** A choice of test execution mode, only varies internally. */ public enum InternalTestExecutionMode { NORMAL