Ensure copts attributes and per_file_copt are handled by LTO backends
A recent bug exposed the fact that copts attributes from BUILD rules are not being propagated to the LTO backend compile invocations for ThinLTO builds. Fix that here by saving this information when creating the compile outputs (which was already saving information for the later LTO steps, such as the correspondence between the full bitcode and the minimized bitcode created by the compile step). Also make this save and therefore propagate to the LTO backends the --per_file_copt applying to that file. Added tests for all these cases (and --copt which were already being applied to the LTO backend but not tested).
Note that nocopts attributes are also not handled properly by LTO backends, however that will be fixed by hlopko@ as part of b/122303926.
RELNOTES: None.
PiperOrigin-RevId: 227859083
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 28d85c0..78806f2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1512,6 +1512,16 @@
return flagsBuilder.build();
}
+ private ImmutableList<String> getCopts(Artifact sourceFile, Label sourceLabel) {
+ ImmutableList.Builder<String> coptsList = ImmutableList.builder();
+ coptsList.addAll(getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()));
+ coptsList.addAll(copts);
+ if (sourceFile != null && sourceLabel != null) {
+ coptsList.addAll(collectPerFileCopts(sourceFile, sourceLabel));
+ }
+ return coptsList.build();
+ }
+
private CcToolchainVariables setupCompileBuildVariables(
CppCompileActionBuilder builder,
Label sourceLabel,
@@ -1523,12 +1533,6 @@
Artifact ltoIndexingFile,
ImmutableMap<String, String> additionalBuildVariables) {
Artifact sourceFile = builder.getSourceFile();
- ImmutableList.Builder<String> userCompileFlags = ImmutableList.builder();
- userCompileFlags.addAll(getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()));
- userCompileFlags.addAll(copts);
- if (sourceFile != null && sourceLabel != null) {
- userCompileFlags.addAll(collectPerFileCopts(sourceFile, sourceLabel));
- }
String dotdFileExecPath = null;
if (builder.getDotdFile() != null) {
dotdFileExecPath = builder.getDotdFile().getSafeExecPath().getPathString();
@@ -1549,7 +1553,7 @@
toPathString(dwoFile),
toPathString(ltoIndexingFile),
ImmutableList.of(),
- userCompileFlags.build(),
+ getCopts(builder.getSourceFile(), sourceLabel),
cppModuleMap,
usePic,
builder.getTempOutputFile(),
@@ -1799,7 +1803,8 @@
result.addPicObjectFile(picAction.getOutputFile());
if (bitcodeOutput) {
- result.addLtoBitcodeFile(picAction.getOutputFile(), ltoIndexingFile);
+ result.addLtoBitcodeFile(
+ picAction.getOutputFile(), ltoIndexingFile, getCopts(sourceArtifact, sourceLabel));
}
}
if (dwoFile != null) {
@@ -1864,7 +1869,8 @@
if (addObject) {
result.addObjectFile(objectFile);
if (bitcodeOutput) {
- result.addLtoBitcodeFile(objectFile, ltoIndexingFile);
+ result.addLtoBitcodeFile(
+ objectFile, ltoIndexingFile, getCopts(sourceArtifact, sourceLabel));
}
}
if (noPicDwoFile != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
index 285b9a1..e0d8878 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
@@ -202,8 +202,9 @@
return this;
}
- public Builder addLtoBitcodeFile(Artifact fullBitcode, Artifact ltoIndexingBitcode) {
- ltoCompilationContext.addBitcodeFile(fullBitcode, ltoIndexingBitcode);
+ public Builder addLtoBitcodeFile(
+ Artifact fullBitcode, Artifact ltoIndexingBitcode, ImmutableList<String> copts) {
+ ltoCompilationContext.addBitcodeFile(fullBitcode, ltoIndexingBitcode, copts);
return this;
}
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 f047422..4823e50 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
@@ -417,11 +417,14 @@
.collect(ImmutableList.toImmutableList());
}
- private List<String> getLtoBackendCommandLineOptions() {
+ private List<String> getLtoBackendCommandLineOptions(
+ Artifact objectFile, ImmutableList<String> copts) {
List<String> argv = new ArrayList<>();
argv.addAll(cppConfiguration.getLinkopts());
- argv.addAll(toolchain.getLegacyCompileOptionsWithCopts());
+ argv.addAll(toolchain.getLegacyCompileOptions());
+ argv.addAll(copts);
argv.addAll(cppConfiguration.getLtoBackendOptions());
+ argv.addAll(collectPerFileLtoBackendOpts(objectFile));
return argv;
}
@@ -459,7 +462,6 @@
}
}
- List<String> argv = getLtoBackendCommandLineOptions();
ImmutableList.Builder<LtoBackendArtifacts> ltoOutputs = ImmutableList.builder();
for (LibraryToLink lib : uniqueLibraries) {
if (!lib.containsObjectFiles()) {
@@ -470,8 +472,9 @@
for (Artifact objectFile : lib.getObjectFiles()) {
if (compiled.contains(objectFile)) {
if (includeLinkStaticInLtoIndexing) {
- List<String> backendArgv = new ArrayList<>(argv);
- backendArgv.addAll(collectPerFileLtoBackendOpts(objectFile));
+ List<String> backendArgv =
+ getLtoBackendCommandLineOptions(
+ objectFile, lib.getLtoCompilationContext().getCopts(objectFile));
LtoBackendArtifacts ltoArtifacts =
createLtoArtifact(
objectFile,
@@ -493,8 +496,9 @@
}
for (LinkerInput input : objectFiles) {
if (this.ltoCompilationContext.containsBitcodeFile(input.getArtifact())) {
- List<String> backendArgv = new ArrayList<>(argv);
- backendArgv.addAll(collectPerFileLtoBackendOpts(input.getArtifact()));
+ List<String> backendArgv =
+ getLtoBackendCommandLineOptions(
+ input.getArtifact(), this.ltoCompilationContext.getCopts(input.getArtifact()));
LtoBackendArtifacts ltoArtifacts =
createLtoArtifact(
input.getArtifact(),
@@ -520,15 +524,14 @@
PathFragment ltoOutputRootPrefix = PathFragment.create(SHARED_NONLTO_BACKEND_ROOT_PREFIX);
- List<String> argv = getLtoBackendCommandLineOptions();
-
ImmutableMap.Builder<Artifact, LtoBackendArtifacts> sharedNonLtoBackends =
ImmutableMap.builder();
for (LinkerInput input : objectFiles) {
if (this.ltoCompilationContext.containsBitcodeFile(input.getArtifact())) {
- List<String> backendArgv = new ArrayList<>(argv);
- backendArgv.addAll(collectPerFileLtoBackendOpts(input.getArtifact()));
+ List<String> backendArgv =
+ getLtoBackendCommandLineOptions(
+ input.getArtifact(), this.ltoCompilationContext.getCopts(input.getArtifact()));
LtoBackendArtifacts ltoArtifacts =
createLtoArtifact(
input.getArtifact(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoCompilationContext.java
index 88f4c3e..586b481 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoCompilationContext.java
@@ -14,14 +14,16 @@
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact;
import java.util.Set;
/**
* Holds information collected for .o bitcode files coming from a ThinLTO C(++) compilation under
- * our control. Currently maps each bitcode file to the corresponding minimized bitcode file that
- * can be used for the LTO indexing step.
+ * our control. Specifically, maps each bitcode file to the corresponding minimized bitcode file
+ * that can be used for the LTO indexing step, as well as to compile flags applying to that
+ * compilation that should also be applied to the LTO backend compilation invocation.
*/
public class LtoCompilationContext {
private final ImmutableMap<Artifact, BitcodeInfo> ltoBitcodeFiles;
@@ -40,15 +42,25 @@
*/
public static class BitcodeInfo {
private final Artifact minimizedBitcode;
+ private final ImmutableList<String> copts;
- public BitcodeInfo(Artifact minimizedBitcode) {
+ public BitcodeInfo(Artifact minimizedBitcode, ImmutableList<String> copts) {
this.minimizedBitcode = minimizedBitcode;
+ this.copts = copts;
}
/** The minimized bitcode file produced by the compile and used by LTO indexing. */
public Artifact getMinimizedBitcode() {
return minimizedBitcode;
}
+
+ /**
+ * The compiler flags used for the compile that should also be used when finishing compilation
+ * during the LTO backend.
+ */
+ public ImmutableList<String> getCopts() {
+ return copts;
+ }
}
/** Builder for LtoCompilationContext. */
@@ -62,9 +74,10 @@
return new LtoCompilationContext(ltoBitcodeFiles.build());
}
- /** Adds a bitcode file with the corresponding minimized bitcode file. */
- public void addBitcodeFile(Artifact fullBitcode, Artifact minimizedBitcode) {
- ltoBitcodeFiles.put(fullBitcode, new BitcodeInfo(minimizedBitcode));
+ /** Adds a bitcode file with the corresponding minimized bitcode file and compiler flags. */
+ public void addBitcodeFile(
+ Artifact fullBitcode, Artifact minimizedBitcode, ImmutableList<String> copts) {
+ ltoBitcodeFiles.put(fullBitcode, new BitcodeInfo(minimizedBitcode, copts));
}
/** Adds in all bitcode files and associated info from another LtoCompilationContext object. */
@@ -89,6 +102,17 @@
return ltoBitcodeFiles.get(fullBitcode).getMinimizedBitcode();
}
+ /**
+ * Gets the compiler flags corresponding to the full bitcode file, or returns an empty list if it
+ * doesn't exist.
+ */
+ public ImmutableList<String> getCopts(Artifact fullBitcode) {
+ if (!containsBitcodeFile(fullBitcode)) {
+ return ImmutableList.of();
+ }
+ return ltoBitcodeFiles.get(fullBitcode).getCopts();
+ }
+
/** Whether the map of bitcode files is empty. */
public boolean isEmpty() {
return ltoBitcodeFiles.isEmpty();
