Make thinlto action command lines standalone
This cl introduces an incompatible flag `--incompatible_make_thinlto_command_lines_standalone`, which when flipped will change Bazel's handling of thinlto:
1) lto-backend action_config now has to be defined in the CcToolchainConfigInfo
2) lto-indexing action is split into:
* lto-index-for-executable
* lto-index-for-dynamic-library
* lto-index-for-nodeps-dynamic-library
3) all these need an action_config too
4) lto-indexing action is completely ignored
5) lto indexing command line is now defined independently of c++-link-executable, c++-link-dynamic-library, and c++-link-nodeps-dynamic-library, flags are no longer reused.
https://github.com/bazelbuild/bazel/issues/6791
RELNOTES: Incompatible flag `--incompatible_make_thinlto_command_lines_standalone` has been added. See https://github.com/bazelbuild/bazel/issues/6791 for details.
PiperOrigin-RevId: 243988905
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index f184592..bedea9a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -122,10 +122,14 @@
CppActionNames.PREPROCESS_ASSEMBLE,
CppActionNames.CLIF_MATCH,
CppActionNames.LINKSTAMP_COMPILE,
- CppActionNames.CC_FLAGS_MAKE_VARIABLE);
+ CppActionNames.CC_FLAGS_MAKE_VARIABLE,
+ CppActionNames.LTO_BACKEND);
public static final ImmutableSet<String> ALL_LINK_ACTIONS =
ImmutableSet.of(
+ CppActionNames.LTO_INDEX_EXECUTABLE,
+ CppActionNames.LTO_INDEX_DYNAMIC_LIBRARY,
+ CppActionNames.LTO_INDEX_NODEPS_DYNAMIC_LIBRARY,
LinkTargetType.EXECUTABLE.getActionName(),
Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(),
Link.LinkTargetType.NODEPS_DYNAMIC_LIBRARY.getActionName());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
index 2e0c3d6..e6a11cb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -979,6 +979,20 @@
getActionConfig(
Joiner.on("\n")
.join(
+ " config_name: 'lto-backend'",
+ " action_name: 'lto-backend'",
+ " tool {",
+ " tool_path: '" + gccToolPath + "'",
+ " }",
+ " implies: 'legacy_compile_flags'",
+ " implies: 'user_compile_flags'",
+ " implies: 'sysroot'",
+ " implies: 'unfiltered_compile_flags'",
+ " implies: 'compiler_input_flags'",
+ " implies: 'compiler_output_flags'")),
+ getActionConfig(
+ Joiner.on("\n")
+ .join(
" config_name: 'c-compile'",
" action_name: 'c-compile'",
" tool {",
@@ -1070,6 +1084,27 @@
getActionConfig(
Joiner.on("\n")
.join(
+ " config_name: 'lto-index-for-executable'",
+ " action_name: 'lto-index-for-executable'",
+ " tool {",
+ " tool_path: '" + gccToolPath + "'",
+ " }",
+ " implies: 'symbol_counts'",
+ " implies: 'strip_debug_symbols'",
+ " implies: 'linkstamps'",
+ " implies: 'output_execpath_flags'",
+ " implies: 'runtime_library_search_directories'",
+ " implies: 'library_search_directories'",
+ " implies: 'libraries_to_link'",
+ " implies: 'force_pic_flags'",
+ " implies: 'user_link_flags'",
+ " implies: 'legacy_link_flags'",
+ " implies: 'linker_param_file'",
+ " implies: 'fission_support'",
+ " implies: 'sysroot'")),
+ getActionConfig(
+ Joiner.on("\n")
+ .join(
" config_name: 'c++-link-nodeps-dynamic-library'",
" action_name: 'c++-link-nodeps-dynamic-library'",
" tool {",
@@ -1093,6 +1128,29 @@
getActionConfig(
Joiner.on("\n")
.join(
+ " config_name: 'lto-index-for-nodeps-dynamic-library'",
+ " action_name: 'lto-index-for-nodeps-dynamic-library'",
+ " tool {",
+ " tool_path: '" + gccToolPath + "'",
+ " }",
+ " implies: 'build_interface_libraries'",
+ " implies: 'dynamic_library_linker_tool'",
+ " implies: 'symbol_counts'",
+ " implies: 'strip_debug_symbols'",
+ " implies: 'shared_flag'",
+ " implies: 'linkstamps'",
+ " implies: 'output_execpath_flags'",
+ " implies: 'runtime_library_search_directories'",
+ " implies: 'library_search_directories'",
+ " implies: 'libraries_to_link'",
+ " implies: 'user_link_flags'",
+ " implies: 'legacy_link_flags'",
+ " implies: 'linker_param_file'",
+ " implies: 'fission_support'",
+ " implies: 'sysroot'")),
+ getActionConfig(
+ Joiner.on("\n")
+ .join(
" config_name: 'c++-link-dynamic-library'",
" action_name: 'c++-link-dynamic-library'",
" tool {",
@@ -1116,6 +1174,29 @@
getActionConfig(
Joiner.on("\n")
.join(
+ " config_name: 'lto-index-for-dynamic-library'",
+ " action_name: 'lto-index-for-dynamic-library'",
+ " tool {",
+ " tool_path: '" + gccToolPath + "'",
+ " }",
+ " implies: 'build_interface_libraries'",
+ " implies: 'dynamic_library_linker_tool'",
+ " implies: 'symbol_counts'",
+ " implies: 'strip_debug_symbols'",
+ " implies: 'shared_flag'",
+ " implies: 'linkstamps'",
+ " implies: 'output_execpath_flags'",
+ " implies: 'runtime_library_search_directories'",
+ " implies: 'library_search_directories'",
+ " implies: 'libraries_to_link'",
+ " implies: 'user_link_flags'",
+ " implies: 'legacy_link_flags'",
+ " implies: 'linker_param_file'",
+ " implies: 'fission_support'",
+ " implies: 'sysroot'")),
+ getActionConfig(
+ Joiner.on("\n")
+ .join(
" config_name: 'c++-link-static-library'",
" action_name: 'c++-link-static-library'",
" tool {",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionNames.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionNames.java
index 6547219..948f204 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionNames.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionNames.java
@@ -69,5 +69,13 @@
public static final String OBJCPP_EXECUTABLE = "objc++-executable";
public static final String LTO_INDEXING = "lto-indexing";
+ /** Name of the action producing thinlto index for dynamic library. */
+ public static final String LTO_INDEX_DYNAMIC_LIBRARY = "lto-index-for-dynamic-library";
+ /** Name of the action producing thinlto index for nodeps dynamic library. */
+ public static final String LTO_INDEX_NODEPS_DYNAMIC_LIBRARY =
+ "lto-index-for-nodeps-dynamic-library";
+ /** Name of the action producing thinlto index for executable binary. */
+ public static final String LTO_INDEX_EXECUTABLE = "lto-index-for-executable";
+
public static final String LTO_BACKEND = "lto-backend";
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 8a91339..8522e37 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -665,4 +665,8 @@
public boolean saveFeatureState() {
return cppOptions.saveFeatureState;
}
+
+ public boolean useStandaloneLtoIndexingCommandLines() {
+ return cppOptions.useStandaloneLtoIndexingCommandLines;
+ }
}
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 86b7e7d..ac21918 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
@@ -191,6 +191,20 @@
/** Returns the action name for purposes of querying the crosstool. */
private String getActionName() {
+ // We check that this action is not lto-indexing, or when it is, it's either for executable
+ // or transitive or nodeps dynamic library.
+ Preconditions.checkArgument(
+ !isLtoIndexing || linkType.isExecutable() || linkType.isDynamicLibrary());
+ if (isLtoIndexing && cppConfiguration.useStandaloneLtoIndexingCommandLines()) {
+ if (linkType.isExecutable()) {
+ return CppActionNames.LTO_INDEX_EXECUTABLE;
+ } else if (linkType.isTransitiveDynamicLibrary()) {
+ return CppActionNames.LTO_INDEX_DYNAMIC_LIBRARY;
+ } else {
+ return CppActionNames.LTO_INDEX_NODEPS_DYNAMIC_LIBRARY;
+ }
+ }
+
return linkType.getActionName();
}
@@ -859,6 +873,13 @@
CcToolchainVariables variables;
try {
+ ImmutableList.Builder<String> userLinkFlags =
+ ImmutableList.<String>builder().addAll(linkopts).addAll(cppConfiguration.getLinkopts());
+
+ if (isLtoIndexing && cppConfiguration.useStandaloneLtoIndexingCommandLines()) {
+ userLinkFlags.addAll(cppConfiguration.getLtoIndexOptions());
+ }
+
variables =
LinkBuildVariables.setupVariables(
getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER),
@@ -875,10 +896,7 @@
featureConfiguration,
useTestOnlyFlags,
isLtoIndexing,
- ImmutableList.<String>builder()
- .addAll(linkopts)
- .addAll(cppConfiguration.getLinkopts())
- .build(),
+ userLinkFlags.build(),
toolchain.getInterfaceSoBuilder().getExecPathString(),
interfaceOutput != null ? interfaceOutput.getExecPathString() : null,
ltoOutputRootPrefix,
@@ -921,6 +939,7 @@
LinkCommandLine.Builder linkCommandLineBuilder =
new LinkCommandLine.Builder()
+ .setActionName(getActionName())
.setLinkerInputArtifacts(expandedLinkerArtifacts)
.setLinkTargetType(linkType)
.setLinkingMode(linkingMode)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index 7600c15..5de5169 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -739,6 +739,20 @@
public boolean dontEnableHostNonhost;
@Option(
+ name = "incompatible_make_thinlto_command_lines_standalone",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If true, Bazel will not reuse C++ link action command lines for lto indexing command "
+ + "lines (see https://github.com/bazelbuild/bazel/issues/6791 for more information).")
+ public boolean useStandaloneLtoIndexingCommandLines;
+
+ @Option(
name = "incompatible_require_ctx_in_configure_features",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
@@ -920,6 +934,7 @@
host.removeLegacyWholeArchive = removeLegacyWholeArchive;
host.dontEnableHostNonhost = dontEnableHostNonhost;
host.requireCtxInConfigureFeatures = requireCtxInConfigureFeatures;
+ host.useStandaloneLtoIndexingCommandLines = useStandaloneLtoIndexingCommandLines;
return host;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
index a029a9d..91647c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
@@ -245,13 +245,18 @@
public String getActionName() {
return actionName;
}
-
- /** Returns true iff this link type is executable */
+
+ /** Returns true iff this link type is executable. */
public boolean isExecutable() {
return (executable == Executable.EXECUTABLE);
}
- /** Returns true iff this link type is a dynamic library or transitive dynamic library */
+ /** Returns true iff this link type is a transitive dynamic library. */
+ public boolean isTransitiveDynamicLibrary() {
+ return this == DYNAMIC_LIBRARY;
+ }
+
+ /** Returns true iff this link type is a dynamic library or transitive dynamic library. */
public boolean isDynamicLibrary() {
return this == NODEPS_DYNAMIC_LIBRARY || this == DYNAMIC_LIBRARY;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java
index a79102e..364a757 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java
@@ -236,7 +236,7 @@
}
Iterable<String> userLinkFlagsWithLtoIndexingIfNeeded;
- if (!isLtoIndexing) {
+ if (!isLtoIndexing || cppConfiguration.useStandaloneLtoIndexingCommandLines()) {
userLinkFlagsWithLtoIndexingIfNeeded = userLinkFlags;
} else {
ImmutableList.Builder<String> opts = ImmutableList.builder();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index f2a5b32..d03d99b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -434,9 +434,9 @@
private CcToolchainVariables variables;
private FeatureConfiguration featureConfiguration;
private boolean doNotSplitLinkingCmdLine;
+ private String actionName;
public LinkCommandLine build() {
-
if (linkTargetType.linkerOrArchiver() == LinkerOrArchiver.ARCHIVER) {
Preconditions.checkArgument(
buildInfoHeaderArtifacts.isEmpty(),
@@ -447,8 +447,6 @@
variables = CcToolchainVariables.EMPTY;
}
- String actionName = linkTargetType.getActionName();
-
return new LinkCommandLine(
actionName,
forcedToolPath,
@@ -557,5 +555,10 @@
this.doNotSplitLinkingCmdLine = true;
return this;
}
+
+ public Builder setActionName(String actionName) {
+ this.actionName = actionName;
+ return this;
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
index c8b2121..04d2d84 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendArtifacts.java
@@ -233,11 +233,6 @@
builder.setProgressMessage("LTO Backend Compile %s", objectFile.getExecPath());
builder.setMnemonic("CcLtoBackendCompile");
- // The command-line doesn't specify the full path to clang++, so we set it in the
- // environment.
- PathFragment compiler = ccToolchain.getToolPathFragment(Tool.GCC, ruleErrorConsumer);
-
- builder.setExecutable(compiler);
CcToolchainVariables.Builder buildVariablesBuilder =
CcToolchainVariables.builder(ccToolchain.getBuildVariables(buildOptions, cppConfiguration));
if (index != null) {
@@ -277,8 +272,24 @@
buildVariablesBuilder.addStringSequenceVariable(
CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), userCompileFlags);
- List<String> execArgs = new ArrayList<>();
CcToolchainVariables buildVariables = buildVariablesBuilder.build();
+
+ if (cppConfiguration.useStandaloneLtoIndexingCommandLines()) {
+ if (!featureConfiguration.actionIsConfigured(CppActionNames.LTO_BACKEND)) {
+ throw ruleErrorConsumer.throwWithRuleError(
+ "Thinlto build is requested, but the C++ toolchain doesn't define an action_config for"
+ + " 'lto-backend' action.");
+ }
+ PathFragment compiler =
+ PathFragment.create(
+ featureConfiguration.getToolPathForAction(CppActionNames.LTO_BACKEND));
+ builder.setExecutable(compiler);
+ } else {
+ PathFragment compiler = ccToolchain.getToolPathFragment(Tool.GCC, ruleErrorConsumer);
+ builder.setExecutable(compiler);
+ }
+
+ List<String> execArgs = new ArrayList<>();
execArgs.addAll(
CppHelper.getCommandLine(
ruleErrorConsumer, featureConfiguration, buildVariables, CppActionNames.LTO_BACKEND));
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl b/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
index 4052bf9..05be8a1 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
@@ -457,6 +457,20 @@
],
),
flag_set(
+ actions = [
+ ACTION_NAMES.lto_index_for_executable,
+ ACTION_NAMES.lto_index_for_dynamic_library,
+ ACTION_NAMES.lto_index_for_nodeps_dynamic_library,
+ ],
+ flag_groups = [
+ flag_group(
+ flags = ["--i_come_from_standalone_lto_index=%{user_link_flags}"],
+ iterate_over = "user_link_flags",
+ expand_if_available = "user_link_flags",
+ ),
+ ],
+ ),
+ flag_set(
actions = [ACTION_NAMES.lto_indexing],
flag_groups = [
flag_group(
diff --git a/tools/build_defs/cc/action_names.bzl b/tools/build_defs/cc/action_names.bzl
index 8fd2026..60e8a5f 100644
--- a/tools/build_defs/cc/action_names.bzl
+++ b/tools/build_defs/cc/action_names.bzl
@@ -43,6 +43,15 @@
# Name of the action producing ThinLto index.
LTO_INDEXING_ACTION_NAME = "lto-indexing"
+# Name of the action producing ThinLto index for executable.
+LTO_INDEX_FOR_EXECUTABLE_ACTION_NAME = "lto-index-for-executable"
+
+# Name of the action producing ThinLto index for dynamic library.
+LTO_INDEX_FOR_DYNAMIC_LIBRARY_ACTION_NAME = "lto-index-for-dynamic-library"
+
+# Name of the action producing ThinLto index for nodeps dynamic library.
+LTO_INDEX_FOR_NODEPS_DYNAMIC_LIBRARY_ACTION_NAME = "lto-index-for-nodeps-dynamic-library"
+
# Name of the action compiling lto bitcodes into native objects.
LTO_BACKEND_ACTION_NAME = "lto-backend"
@@ -95,6 +104,9 @@
preprocess_assemble = PREPROCESS_ASSEMBLE_ACTION_NAME,
lto_indexing = LTO_INDEXING_ACTION_NAME,
lto_backend = LTO_BACKEND_ACTION_NAME,
+ lto_index_for_executable = LTO_INDEX_FOR_EXECUTABLE_ACTION_NAME,
+ lto_index_for_dynamic_library = LTO_INDEX_FOR_DYNAMIC_LIBRARY_ACTION_NAME,
+ lto_index_for_nodeps_dynamic_library = LTO_INDEX_FOR_NODEPS_DYNAMIC_LIBRARY_ACTION_NAME,
cpp_link_executable = CPP_LINK_EXECUTABLE_ACTION_NAME,
cpp_link_dynamic_library = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME,
cpp_link_nodeps_dynamic_library = CPP_LINK_NODEPS_DYNAMIC_LIBRARY_ACTION_NAME,
