diff options
author | 2015-10-06 15:34:20 +0000 | |
---|---|---|
committer | 2015-10-07 07:09:45 +0000 | |
commit | ad1114fa3bda235d9badeb907a07273a665dc99d (patch) | |
tree | 4234cb232c9caa475ff2239ab922e1eedb520297 /src | |
parent | 4def9d03408cf461d3eec452170ed5990ed6fdf4 (diff) |
Collect .o files compiled from C(++), and skip the rest for the LTO backend step.
This should make ThinLTO work with .o files that are not generated by
compiling C++ in a cc_library()
--
MOS_MIGRATED_REVID=104764111
Diffstat (limited to 'src')
9 files changed, 169 insertions, 54 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 6fb4a6dc74..3786fcc1a9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -343,14 +343,15 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // Determine the object files to link in. boolean usePic = CppHelper.usePic(context, !isLinkShared(context)); - Iterable<Artifact> compiledObjectFiles = compilationOutputs.getObjectFiles(usePic); + Iterable<Artifact> objectFiles = compilationOutputs.getObjectFiles(usePic); if (fake) { - builder.addFakeNonLibraryInputs(compiledObjectFiles); + builder.addFakeNonLibraryInputs(objectFiles); } else { - builder.addNonLibraryInputs(compiledObjectFiles); + builder.addNonLibraryInputs(objectFiles); } + builder.addLTOBitcodeFiles(compilationOutputs.getLtoBitcodeFiles()); builder.addNonLibraryInputs(common.getLinkerScripts()); // Determine the libraries to link in. 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 f7ff4d8227..67162c20d7 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 @@ -40,6 +40,11 @@ public class CcCompilationOutputs { private final ImmutableList<Artifact> picObjectFiles; /** + * All .o files coming from a C(++) compilation under our control. + */ + private final ImmutableList<Artifact> ltoBitcodeFiles; + + /** * All .dwo files built by the target, corresponding to .o outputs. */ private final ImmutableList<Artifact> dwoFiles; @@ -61,13 +66,19 @@ public class CcCompilationOutputs { private final List<IncludeScannable> lipoScannables; - private CcCompilationOutputs(ImmutableList<Artifact> objectFiles, - ImmutableList<Artifact> picObjectFiles, ImmutableList<Artifact> dwoFiles, - ImmutableList<Artifact> picDwoFiles, NestedSet<Artifact> temps, + private CcCompilationOutputs( + ImmutableList<Artifact> objectFiles, + ImmutableList<Artifact> picObjectFiles, + ImmutableList<Artifact> ltoBitcodeFiles, + + ImmutableList<Artifact> dwoFiles, + ImmutableList<Artifact> picDwoFiles, + NestedSet<Artifact> temps, ImmutableList<Artifact> headerTokenFiles, ImmutableList<IncludeScannable> lipoScannables) { this.objectFiles = objectFiles; this.picObjectFiles = picObjectFiles; + this.ltoBitcodeFiles = ltoBitcodeFiles; this.dwoFiles = dwoFiles; this.picDwoFiles = picDwoFiles; this.temps = temps; @@ -92,6 +103,13 @@ public class CcCompilationOutputs { } /** + * Returns unmodifiable view of object files resulting from compilation. + */ + public ImmutableList<Artifact> getLtoBitcodeFiles() { + return ltoBitcodeFiles; + } + + /** * Returns an unmodifiable view of the .dwo files set. */ public ImmutableList<Artifact> getDwoFiles() { @@ -130,6 +148,7 @@ public class CcCompilationOutputs { public static final class Builder { private final Set<Artifact> objectFiles = new LinkedHashSet<>(); private final Set<Artifact> picObjectFiles = new LinkedHashSet<>(); + private final Set<Artifact> ltoBitcodeFiles = new LinkedHashSet<>(); private final Set<Artifact> dwoFiles = new LinkedHashSet<>(); private final Set<Artifact> picDwoFiles = new LinkedHashSet<>(); private final NestedSetBuilder<Artifact> temps = NestedSetBuilder.stableOrder(); @@ -137,9 +156,13 @@ public class CcCompilationOutputs { private final List<IncludeScannable> lipoScannables = new ArrayList<>(); public CcCompilationOutputs build() { - return new CcCompilationOutputs(ImmutableList.copyOf(objectFiles), - ImmutableList.copyOf(picObjectFiles), ImmutableList.copyOf(dwoFiles), - ImmutableList.copyOf(picDwoFiles), temps.build(), + return new CcCompilationOutputs( + ImmutableList.copyOf(objectFiles), + ImmutableList.copyOf(picObjectFiles), + ImmutableList.copyOf(ltoBitcodeFiles), + ImmutableList.copyOf(dwoFiles), + ImmutableList.copyOf(picDwoFiles), + temps.build(), ImmutableList.copyOf(headerTokenFiles), ImmutableList.copyOf(lipoScannables)); } @@ -176,6 +199,16 @@ public class CcCompilationOutputs { return this; } + public Builder addLTOBitcodeFile(Artifact a) { + ltoBitcodeFiles.add(a); + return this; + } + + public Builder addLTOBitcodeFile(Iterable<Artifact> artifacts) { + Iterables.addAll(ltoBitcodeFiles, artifacts); + return this; + } + public Builder addPicObjectFiles(Iterable<Artifact> artifacts) { Iterables.addAll(picObjectFiles, artifacts); return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index ac06521f8c..8ff2114b92 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -648,11 +648,13 @@ public final class CcLibraryHelper { CcCompilationOutputs ccOutputs = model.createCcCompileActions(); if (!objectFiles.isEmpty() || !picObjectFiles.isEmpty()) { // Merge the pre-compiled object files into the compiler outputs. - ccOutputs = new CcCompilationOutputs.Builder() - .merge(ccOutputs) - .addObjectFiles(objectFiles) - .addPicObjectFiles(picObjectFiles) - .build(); + ccOutputs = + new CcCompilationOutputs.Builder() + .merge(ccOutputs) + .addLTOBitcodeFile(ccOutputs.getLtoBitcodeFiles()) + .addObjectFiles(objectFiles) + .addPicObjectFiles(picObjectFiles) + .build(); } // Create link actions (only if there are object files or if explicitly requested). diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java index 7574839d3d..478759ab34 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.util.FileTypeSet; import java.util.List; import java.util.regex.Pattern; @@ -25,6 +26,11 @@ import java.util.regex.Pattern; public final class CppFileTypes { public static final FileType CPP_SOURCE = FileType.of(".cc", ".cpp", ".cxx", ".c++", ".C"); public static final FileType C_SOURCE = FileType.of(".c"); + + // Filetypes that generate LLVM bitcode when -flto is specified. + public static final FileTypeSet LTO_SOURCE = + FileTypeSet.of(CppFileTypes.CPP_SOURCE, CppFileTypes.C_SOURCE); + public static final FileType CPP_HEADER = FileType.of(".h", ".hh", ".hpp", ".hxx", ".inc"); public static final FileType CPP_TEXTUAL_INCLUDE = FileType.of(".inc"); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java index d11fd3c3aa..4d412e983c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java @@ -64,6 +64,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -517,7 +518,6 @@ public final class CppLinkAction extends AbstractAction { private final AnalysisEnvironment analysisEnvironment; private final Artifact output; - @Nullable private PathFragment interfaceOutputPath; // can be null for CppLinkAction.createTestBuilder() @Nullable private final CcToolchainProvider toolchain; private Artifact interfaceOutput; @@ -539,6 +539,8 @@ public final class CppLinkAction extends AbstractAction { private final List<String> linkopts = new ArrayList<>(); private LinkTargetType linkType = LinkTargetType.STATIC_LIBRARY; private LinkStaticness linkStaticness = LinkStaticness.FULLY_STATIC; + private List<Artifact> ltoBitcodeFiles = new ArrayList<>(); + private boolean fake; private boolean isNativeDeps; private boolean useTestOnlyFlags; @@ -631,11 +633,20 @@ public final class CppLinkAction extends AbstractAction { private Iterable<LTOBackendArtifacts> createLTOArtifacts( PathFragment ltoOutputRootPrefix, NestedSet<LibraryToLink> uniqueLibraries) { + Set<Artifact> compiled = new LinkedHashSet<>(); + for (LibraryToLink lib : uniqueLibraries) { + Iterables.addAll(compiled, lib.getLTOBitcodeFiles()); + } + // This flattens the set of object files, so for M binaries and N .o files, // this is O(M*N). If we had a nested set of .o files, we could have O(M + N) instead. NestedSetBuilder<Artifact> bitcodeBuilder = NestedSetBuilder.stableOrder(); for (LibraryToLink lib : uniqueLibraries) { - bitcodeBuilder.addAll(lib.getObjectFiles()); + for (Artifact a : lib.getObjectFiles()) { + if (compiled.contains(a)) { + bitcodeBuilder.add(a); + } + } } for (LinkerInput input : nonLibraries) { // This relies on file naming conventions. It would be less fragile to have a dedicated @@ -712,11 +723,12 @@ public final class CppLinkAction extends AbstractAction { : ruleContext.getFeatures(); final LibraryToLink outputLibrary = - LinkerInputs.newInputLibrary(output, filteredNonLibraryArtifacts); + LinkerInputs.newInputLibrary(output, filteredNonLibraryArtifacts, this.ltoBitcodeFiles); final LibraryToLink interfaceOutputLibrary = (interfaceOutput == null) ? null - : LinkerInputs.newInputLibrary(interfaceOutput, filteredNonLibraryArtifacts); + : LinkerInputs.newInputLibrary( + interfaceOutput, filteredNonLibraryArtifacts, this.ltoBitcodeFiles); final ImmutableMap<Artifact, Artifact> linkstampMap = mapLinkstampsToOutputs(linkstamps, ruleContext, output, linkArtifactFactory); @@ -815,22 +827,39 @@ public final class CppLinkAction extends AbstractAction { LinkerInputs.toLibraryArtifacts( Link.mergeInputsDependencies( uniqueLibraries, needWholeArchive, cppConfiguration.archiveType())); - + Iterable<Artifact> expandedNonLibraryInputs = LinkerInputs.toLibraryArtifacts(nonLibraries); if (!isLTOIndexing && allLTOArtifacts != null) { - // This is the real link, rename the inputs. - List<Artifact> renamed = new ArrayList<>(); + // We are doing LTO, and this is the real link, so substitute + // the LTO bitcode files with the real object files they were translated into. + Map<Artifact, Artifact> ltoMapping = new HashMap<>(); for (LTOBackendArtifacts a : allLTOArtifacts) { - renamed.add(a.getObjectFile()); + ltoMapping.put(a.getBitcodeFile(), a.getObjectFile()); + } + + // Handle libraries. + List<Artifact> renamedInputs = new ArrayList<>(); + for (Artifact a : expandedInputs) { + Artifact renamed = ltoMapping.get(a); + renamedInputs.add(renamed == null ? a : renamed); + } + expandedInputs = renamedInputs; + + // Handle non-libraries. + List<Artifact> renamedNonLibraryInputs = new ArrayList<>(); + for (Artifact a : expandedNonLibraryInputs) { + Artifact renamed = ltoMapping.get(a); + renamedNonLibraryInputs.add(renamed == null ? a : renamed); } - expandedInputs = renamed; + expandedNonLibraryInputs = renamedNonLibraryInputs; } // getPrimaryInput returns the first element, and that is a public interface - therefore the // order here is important. - IterablesChain.Builder<Artifact> inputsBuilder = IterablesChain.<Artifact>builder() - .add(ImmutableList.copyOf(LinkerInputs.toLibraryArtifacts(nonLibraries))) - .add(dependencyInputsBuilder.build()) - .add(ImmutableIterable.from(expandedInputs)); + IterablesChain.Builder<Artifact> inputsBuilder = + IterablesChain.<Artifact>builder() + .add(ImmutableList.copyOf(expandedNonLibraryInputs)) + .add(dependencyInputsBuilder.build()) + .add(ImmutableIterable.from(expandedInputs)); if (linkCommandLine.getParamFile() != null) { inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile())); @@ -989,6 +1018,13 @@ public final class CppLinkAction extends AbstractAction { this.nonLibraries.add(input); } + public Builder addLTOBitcodeFiles(Iterable<Artifact> files) { + for (Artifact a : files) { + ltoBitcodeFiles.add(a); + } + return this; + } + /** * Adds a single artifact to the set of inputs (C++ source files, header files, etc). Artifacts * that are not of recognized types will be used for dependency checking but will not be passed diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 262dd3794e..fdb0b348a0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -484,7 +484,12 @@ public final class CppModel { CppCompileAction picAction = picBuilder.build(); env.registerAction(picAction); if (addObject) { - result.addPicObjectFile(picAction.getOutputFile()); + result.addPicObjectFile(picAction.getOutputFile()); + + if (featureConfiguration.isEnabled(CppRuleClasses.THIN_LTO) + && CppFileTypes.LTO_SOURCE.matches(sourceArtifact.getFilename())) { + result.addLTOBitcodeFile(picAction.getOutputFile()); + } } if (picAction.getDwoFile() != null) { // Host targets don't produce .dwo files. @@ -511,8 +516,12 @@ public final class CppModel { if (maySaveTemps) { result.addTemps( - createTempsActions(sourceArtifact, outputName, builder, /*usePic=*/false, - ccRelativeName)); + createTempsActions( + sourceArtifact, + outputName, + builder, + /*usePic=*/ false, + ccRelativeName)); } semantics.finalizeCompileActionBuilder(ruleContext, builder); @@ -521,6 +530,10 @@ public final class CppModel { Artifact objectFile = compileAction.getOutputFile(); if (addObject) { result.addObjectFile(objectFile); + if (featureConfiguration.isEnabled(CppRuleClasses.THIN_LTO) + && CppFileTypes.LTO_SOURCE.matches(sourceArtifact.getFilename())) { + result.addLTOBitcodeFile(objectFile); + } } if (compileAction.getDwoFile() != null) { // Host targets don't produce .dwo files. @@ -603,12 +616,14 @@ public final class CppModel { // Presumably, it is done this way because the .a file is an implicit output of every cc_library // rule, so we can't use ".pic.a" that in the always-PIC case. Artifact linkedArtifact = CppHelper.getLinkedArtifact(ruleContext, linkType); - CppLinkAction maybePicAction = newLinkActionBuilder(linkedArtifact) - .addNonLibraryInputs(ccOutputs.getObjectFiles(usePicForBinaries)) - .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) - .setLinkType(linkType) - .setLinkStaticness(LinkStaticness.FULLY_STATIC) - .build(); + CppLinkAction maybePicAction = + newLinkActionBuilder(linkedArtifact) + .addNonLibraryInputs(ccOutputs.getObjectFiles(usePicForBinaries)) + .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) + .addLTOBitcodeFiles(ccOutputs.getLtoBitcodeFiles()) + .setLinkType(linkType) + .setLinkStaticness(LinkStaticness.FULLY_STATIC) + .build(); env.registerAction(maybePicAction); result.addStaticLibrary(maybePicAction.getOutputLibrary()); @@ -621,12 +636,14 @@ public final class CppModel { : LinkTargetType.PIC_STATIC_LIBRARY; Artifact picArtifact = CppHelper.getLinkedArtifact(ruleContext, picLinkType); - CppLinkAction picAction = newLinkActionBuilder(picArtifact) - .addNonLibraryInputs(ccOutputs.getObjectFiles(true)) - .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) - .setLinkType(picLinkType) - .setLinkStaticness(LinkStaticness.FULLY_STATIC) - .build(); + CppLinkAction picAction = + newLinkActionBuilder(picArtifact) + .addNonLibraryInputs(ccOutputs.getObjectFiles(true)) + .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) + .addLTOBitcodeFiles(ccOutputs.getLtoBitcodeFiles()) + .setLinkType(picLinkType) + .setLinkStaticness(LinkStaticness.FULLY_STATIC) + .build(); env.registerAction(picAction); result.addPicStaticLibrary(picAction.getOutputLibrary()); } 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 eb135726d8..78ee47bf9f 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 @@ -120,7 +120,7 @@ public final class LTOBackendArtifacts { builder.addTransitiveInputs(CppHelper.getToolchain(ruleContext).getLink()); builder.addOutput(objectFile); - builder.setProgressMessage("LTO Backend Compile"); + builder.setProgressMessage("LTO Backend Compile " + objectFile.getFilename()); builder.setMnemonic("CcLtoBackendCompile"); // The command-line doesn't specify the full path to clang++, so we set it in the 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 b46cca78c0..64f779173d 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 @@ -938,15 +938,15 @@ public final class LinkCommandLine extends CommandLine { if (ltoMap != null) { Artifact backend = ltoMap.remove(member); - if (backend == null) { - System.err.println( - "LTO backend file missing for " + member + " already did: " + options); - backend = member; + if (backend != null) { + // If the backend artifact is missing, we can't print a warning because this may + // happen normally, due libraries that list .o files explicitly, or generate .o + // files from assembler. + member = backend; } - options.add(backend.getExecPathString()); - } else { - options.add(member.getExecPathString()); } + + options.add(member.getExecPathString()); } options.add("-Wl,--end-lib"); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java index e3875be2c1..21e4146c63 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkerInputs.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.base.Function; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.CollectionUtils; @@ -113,6 +114,8 @@ public abstract class LinkerInputs { * Returns whether the library is a solib symlink. */ boolean isSolibSymlink(); + + Iterable<Artifact> getLTOBitcodeFiles(); } /** @@ -146,6 +149,11 @@ public abstract class LinkerInputs { } @Override + public Iterable<Artifact> getLTOBitcodeFiles() { + return ImmutableList.<Artifact>of(); + } + + @Override public boolean isFake() { return false; } @@ -195,10 +203,16 @@ public abstract class LinkerInputs { private static class CompoundLibraryToLink implements LibraryToLink { private final Artifact libraryArtifact; private final Iterable<Artifact> objectFiles; + private final Iterable<Artifact> ltoBitcodeFiles; - private CompoundLibraryToLink(Artifact libraryArtifact, Iterable<Artifact> objectFiles) { + private CompoundLibraryToLink( + Artifact libraryArtifact, + Iterable<Artifact> objectFiles, + Iterable<Artifact> ltoBitcodeFiles) { this.libraryArtifact = Preconditions.checkNotNull(libraryArtifact); this.objectFiles = objectFiles == null ? null : CollectionUtils.makeImmutable(objectFiles); + this.ltoBitcodeFiles = + (ltoBitcodeFiles == null) ? null : CollectionUtils.makeImmutable(ltoBitcodeFiles); } @Override @@ -233,6 +247,11 @@ public abstract class LinkerInputs { } @Override + public Iterable<Artifact> getLTOBitcodeFiles() { + return ltoBitcodeFiles; + } + + @Override public boolean equals(Object that) { if (this == that) { return true; @@ -318,14 +337,15 @@ public abstract class LinkerInputs { // Preconditions.checkArgument( // !(artifact.getGeneratingAction() instanceof CppLinkAction) || // !Link.ARCHIVE_LIBRARY_FILETYPES.contains(artifact.getFileType())); - return new CompoundLibraryToLink(artifact, null); + return new CompoundLibraryToLink(artifact, null, null); } /** * Creates a library to link with the specified object files. */ - public static LibraryToLink newInputLibrary(Artifact library, Iterable<Artifact> objectFiles) { - return new CompoundLibraryToLink(library, objectFiles); + public static LibraryToLink newInputLibrary( + Artifact library, Iterable<Artifact> objectFiles, Iterable<Artifact> ltoBitcodeFiles) { + return new CompoundLibraryToLink(library, objectFiles, ltoBitcodeFiles); } private static final Function<LibraryToLink, Artifact> LIBRARY_TO_NON_SOLIB = |