diff options
15 files changed, 148 insertions, 69 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 d0b29958b5..1278aad640 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 @@ -163,6 +163,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { ruleContext.checkSrcsSamePackage(true); CcCommon common = new CcCommon(ruleContext); CcToolchainProvider ccToolchain = common.getToolchain(); + FdoSupportProvider fdoSupport = common.getFdoSupport(); FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext, ccToolchain); CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); @@ -185,7 +186,12 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { && linkStaticness == LinkStaticness.DYNAMIC; CcLibraryHelper helper = - new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain) + new CcLibraryHelper( + ruleContext, + semantics, + featureConfiguration, + ccToolchain, + fdoSupport) .fromCommon(common) .addSources(common.getSources()) .addDeps(ImmutableList.of(CppHelper.mallocForTarget(ruleContext))) @@ -230,6 +236,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { determineLinkerArguments( ruleContext, ccToolchain, + fdoSupport.getFdoSupport(), common, precompiledFiles, info, @@ -289,6 +296,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { ruleContext, featureConfiguration, ccToolchain, + fdoSupport, usePic, /*generateDwo=*/ cppConfiguration.useFission()); } @@ -436,6 +444,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { private static CppLinkActionBuilder determineLinkerArguments( RuleContext context, CcToolchainProvider toolchain, + FdoSupport fdoSupport, CcCommon common, PrecompiledFiles precompiledFiles, CcLibraryHelper.Info info, @@ -447,7 +456,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { boolean linkCompileOutputSeparately) throws InterruptedException { CppLinkActionBuilder builder = - new CppLinkActionBuilder(context, binary, toolchain) + new CppLinkActionBuilder(context, binary, toolchain, fdoSupport) .setCrosstoolInputs(toolchain.getLink()) .addNonCodeInputs(compilationPrerequisites); 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 5989d605b8..597e5dd1a8 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 @@ -97,11 +97,15 @@ public final class CcCommon { private final CcToolchainProvider ccToolchain; + private final FdoSupportProvider fdoSupport; + public CcCommon(RuleContext ruleContext) { this.ruleContext = ruleContext; this.cppConfiguration = ruleContext.getFragment(CppConfiguration.class); this.ccToolchain = Preconditions.checkNotNull(CppHelper.getToolchain(ruleContext, ":cc_toolchain")); + this.fdoSupport = + Preconditions.checkNotNull(CppHelper.getFdoSupport(ruleContext, ":cc_toolchain")); } /** @@ -194,7 +198,7 @@ public final class CcCommon { } public TransitiveLipoInfoProvider collectTransitiveLipoLabels(CcCompilationOutputs outputs) { - if (CppHelper.getFdoSupport(ruleContext).getFdoRoot() == null + if (fdoSupport.getFdoSupport().getFdoRoot() == null || !cppConfiguration.isLipoContextCollector()) { return TransitiveLipoInfoProvider.EMPTY; } @@ -283,6 +287,13 @@ public final class CcCommon { } /** + * Returns the C++ FDO optimization support provider. + */ + public FdoSupportProvider getFdoSupport() { + return fdoSupport; + } + + /** * Returns the files from headers and does some sanity checks. Note that this method reports * warnings to the {@link RuleContext} as a side effect, and so should only be called once for any * given rule. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java index 4ca632a37e..e2288523a7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcIncLibrary.java @@ -126,9 +126,9 @@ public abstract class CcIncLibrary implements RuleConfiguredTargetFactory { ImmutableSortedMap<Artifact, Artifact> virtualArtifactMap = virtualArtifactMapBuilder.build(); ruleContext.registerAction( new CreateIncSymlinkAction(ruleContext.getActionOwner(), virtualArtifactMap, includeRoot)); - + FdoSupportProvider fdoSupport = CppHelper.getFdoSupport(ruleContext, ":cc_toolchain"); CcLibraryHelper.Info info = - new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain) + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) .addIncludeDirs(Arrays.asList(includePath)) .addPublicHeaders(virtualArtifactMap.keySet()) .addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index e99c12e243..30ef96ef89 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -109,6 +109,7 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { throws RuleErrorException, InterruptedException { final CcCommon common = new CcCommon(ruleContext); CcToolchainProvider ccToolchain = common.getToolchain(); + FdoSupportProvider fdoSupport = common.getFdoSupport(); FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext, ccToolchain); PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext); @@ -119,7 +120,7 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { } CcLibraryHelper helper = - new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain) + new CcLibraryHelper(ruleContext, semantics, featureConfiguration, ccToolchain, fdoSupport) .fromCommon(common) .addLinkopts(common.getLinkopts()) .addSources(common.getSources()) 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 9d5295764e..5b796fa1e0 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 @@ -294,6 +294,7 @@ public final class CcLibraryHelper { private final FeatureConfiguration featureConfiguration; private CcToolchainProvider ccToolchain; + private final FdoSupportProvider fdoSupport; /** * Creates a CcLibraryHelper. @@ -303,18 +304,21 @@ public final class CcLibraryHelper { * @param featureConfiguration activated features and action configs for the build * @param sourceCatagory the candidate source types for the build * @param ccToolchain the C++ toolchain provider for the build + * @param fdoSupport the C++ FDO optimization support provider for the build */ public CcLibraryHelper( RuleContext ruleContext, CppSemantics semantics, FeatureConfiguration featureConfiguration, SourceCategory sourceCatagory, - CcToolchainProvider ccToolchain) { + CcToolchainProvider ccToolchain, + FdoSupportProvider fdoSupport) { this.ruleContext = Preconditions.checkNotNull(ruleContext); this.semantics = Preconditions.checkNotNull(semantics); this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration); this.sourceCategory = Preconditions.checkNotNull(sourceCatagory); this.ccToolchain = Preconditions.checkNotNull(ccToolchain); + this.fdoSupport = Preconditions.checkNotNull(fdoSupport); } /** @@ -324,11 +328,12 @@ public final class CcLibraryHelper { * @param semantics CppSemantics for the build * @param featureConfiguration activated features and action configs for the build * @param ccToolchain the C++ toolchain provider for the build + * @param fdoSupport the C++ FDO optimization support provider for the build */ public CcLibraryHelper( RuleContext ruleContext, CppSemantics semantics, FeatureConfiguration featureConfiguration, - CcToolchainProvider ccToolchain) { - this(ruleContext, semantics, featureConfiguration, SourceCategory.CC, ccToolchain); + CcToolchainProvider ccToolchain, FdoSupportProvider fdoSupport) { + this(ruleContext, semantics, featureConfiguration, SourceCategory.CC, ccToolchain, fdoSupport); } /** Sets fields that overlap for cc_library and cc_binary rules. */ @@ -1067,7 +1072,7 @@ public final class CcLibraryHelper { * Creates the C/C++ compilation action creator. */ private CppModel initializeCppModel() { - return new CppModel(ruleContext, semantics, ccToolchain) + return new CppModel(ruleContext, semantics, ccToolchain, fdoSupport) .addCompilationUnitSources(compilationUnitSources) .addCopts(copts) .setLinkTargetType(linkType) @@ -1384,7 +1389,7 @@ public final class CcLibraryHelper { } private TransitiveLipoInfoProvider collectTransitiveLipoInfo(CcCompilationOutputs outputs) { - if (CppHelper.getFdoSupport(ruleContext).getFdoRoot() == null) { + if (fdoSupport.getFdoSupport().getFdoRoot() == null) { return TransitiveLipoInfoProvider.EMPTY; } NestedSetBuilder<IncludeScannable> scannableBuilder = NestedSetBuilder.stableOrder(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 5228588d91..8f7ae9a5e5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -216,11 +216,11 @@ public class CppHelper { return false; } - @Nullable public static FdoSupport getFdoSupport(RuleContext ruleContext) { + @Nullable public static FdoSupportProvider getFdoSupport(RuleContext ruleContext, + String ccToolchainAttribute) { return ruleContext - .getPrerequisite(":cc_toolchain", Mode.TARGET) - .getProvider(FdoSupportProvider.class) - .getFdoSupport(); + .getPrerequisite(ccToolchainAttribute, Mode.TARGET) + .getProvider(FdoSupportProvider.class); } public static NestedSet<Pair<String, String>> getCoverageEnvironmentIfNeeded( @@ -507,9 +507,9 @@ public class CppHelper { /** * Returns the FDO build subtype. */ - public static String getFdoBuildStamp(RuleContext ruleContext) { + public static String getFdoBuildStamp(RuleContext ruleContext, FdoSupport fdoSupport) { CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); - if (getFdoSupport(ruleContext).isAutoFdoEnabled()) { + if (fdoSupport.isAutoFdoEnabled()) { return (cppConfiguration.getLipoMode() == LipoMode.BINARY) ? "ALIPO" : "AFDO"; } if (cppConfiguration.isFdo()) { 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 ffe579062a..2746c64afe 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 @@ -143,6 +143,7 @@ public class CppLinkActionBuilder { // can be null for CppLinkAction.createTestBuilder() @Nullable private final CcToolchainProvider toolchain; + private final FdoSupport fdoSupport; private Artifact interfaceOutput; private Artifact symbolCounts; private PathFragment runtimeSolibDir; @@ -186,15 +187,17 @@ public class CppLinkActionBuilder { * @param ruleContext the rule that owns the action * @param output the output artifact * @param toolchain the C++ toolchain provider + * @param the C++ FDO optimization support */ public CppLinkActionBuilder(RuleContext ruleContext, Artifact output, - CcToolchainProvider toolchain) { + CcToolchainProvider toolchain, FdoSupport fdoSupport) { this( ruleContext, output, ruleContext.getConfiguration(), ruleContext.getAnalysisEnvironment(), - toolchain); + toolchain, + fdoSupport); } /** @@ -204,13 +207,16 @@ public class CppLinkActionBuilder { * @param output the output artifact * @param configuration build configuration * @param toolchain C++ toolchain provider + * @param the C++ FDO optimization support */ public CppLinkActionBuilder( RuleContext ruleContext, Artifact output, BuildConfiguration configuration, - CcToolchainProvider toolchain) { - this(ruleContext, output, configuration, ruleContext.getAnalysisEnvironment(), toolchain); + CcToolchainProvider toolchain, + FdoSupport fdoSupport) { + this(ruleContext, output, configuration, ruleContext.getAnalysisEnvironment(), toolchain, + fdoSupport); } /** @@ -220,19 +226,23 @@ public class CppLinkActionBuilder { * @param output the output artifact * @param configuration the configuration used to determine the tool chain and the default link * options + * @param toolchain the C++ toolchain provider + * @param the C++ FDO optimization support */ private CppLinkActionBuilder( @Nullable RuleContext ruleContext, Artifact output, BuildConfiguration configuration, AnalysisEnvironment analysisEnvironment, - CcToolchainProvider toolchain) { + CcToolchainProvider toolchain, + FdoSupport fdoSupport) { this.ruleContext = ruleContext; this.analysisEnvironment = Preconditions.checkNotNull(analysisEnvironment); this.output = Preconditions.checkNotNull(output); this.configuration = Preconditions.checkNotNull(configuration); this.cppConfiguration = configuration.getFragment(CppConfiguration.class); this.toolchain = toolchain; + this.fdoSupport = fdoSupport; if (cppConfiguration.supportsEmbeddedRuntimes() && toolchain != null) { runtimeSolibDir = toolchain.getDynamicRuntimeSolibDir(); } @@ -247,13 +257,15 @@ public class CppLinkActionBuilder { * @param linkContext an immutable CppLinkAction.Context from the original builder * @param configuration build configuration * @param toolchain the C++ toolchain provider + * @param the C++ FDO optimization support */ public CppLinkActionBuilder( RuleContext ruleContext, Artifact output, Context linkContext, BuildConfiguration configuration, - CcToolchainProvider toolchain) { + CcToolchainProvider toolchain, + FdoSupport fdoSupport) { // These Builder-only fields get set in the constructor: // ruleContext, analysisEnvironment, outputPath, configuration, runtimeSolibDir this( @@ -261,7 +273,8 @@ public class CppLinkActionBuilder { output, configuration, ruleContext.getAnalysisEnvironment(), - toolchain); + toolchain, + fdoSupport); Preconditions.checkNotNull(linkContext); // All linkContext fields should be transferred to this Builder. @@ -655,6 +668,7 @@ public class CppLinkActionBuilder { .setUseTestOnlyFlags(useTestOnlyFlags) .setParamFile(paramFile) .setToolchain(toolchain) + .setFdoSupport(fdoSupport) .setBuildVariables(buildVariables) .setToolPath(getToolPath()) .setFeatureConfiguration(featureConfiguration); @@ -1405,7 +1419,7 @@ public class CppLinkActionBuilder { buildVariables .addAllStringVariables(toolchain.getBuildVariables()) .build(); - CppHelper.getFdoSupport(ruleContext).getLinkOptions(featureConfiguration, buildVariables); + fdoSupport.getLinkOptions(featureConfiguration, buildVariables); } private boolean isLTOIndexing() { 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 7960baca9d..c1d639bc83 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 @@ -87,13 +87,15 @@ public final class CppModel { private Artifact soImplArtifact; private FeatureConfiguration featureConfiguration; private List<VariablesExtension> variablesExtensions = new ArrayList<>(); - private CcToolchainProvider ccToolchain; + private final CcToolchainProvider ccToolchain; + private final FdoSupportProvider fdoSupport; public CppModel(RuleContext ruleContext, CppSemantics semantics, - CcToolchainProvider ccToolchain) { + CcToolchainProvider ccToolchain, FdoSupportProvider fdoSupport) { this.ruleContext = Preconditions.checkNotNull(ruleContext); this.semantics = semantics; this.ccToolchain = Preconditions.checkNotNull(ccToolchain); + this.fdoSupport = Preconditions.checkNotNull(fdoSupport); configuration = ruleContext.getConfiguration(); cppConfiguration = ruleContext.getFragment(CppConfiguration.class); } @@ -415,14 +417,15 @@ public final class CppModel { } if (featureConfiguration.isEnabled(CppRuleClasses.PREPROCESSOR_DEFINES)) { - String fdoBuildStamp = CppHelper.getFdoBuildStamp(ruleContext); + String fdoBuildStamp = CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()); ImmutableList<String> defines; if (fdoBuildStamp != null) { // Stamp FDO builds with FDO subtype string defines = ImmutableList.<String>builder() .addAll(builderContext.getDefines()) .add(CppConfiguration.FDO_STAMP_MACRO - + "=\"" + CppHelper.getFdoBuildStamp(ruleContext) + "\"") + + "=\"" + CppHelper.getFdoBuildStamp( + ruleContext, fdoSupport.getFdoSupport()) + "\"") .build(); } else { defines = builderContext.getDefines(); @@ -439,8 +442,9 @@ public final class CppModel { } if (ccRelativeName != null) { - CppHelper.getFdoSupport(ruleContext).configureCompilation(builder, buildVariables, - ruleContext, ccRelativeName, autoFdoImportPath, usePic, featureConfiguration); + fdoSupport.getFdoSupport().configureCompilation( + builder, buildVariables, ruleContext, ccRelativeName, autoFdoImportPath, usePic, + featureConfiguration, fdoSupport); } if (gcnoFile != null) { buildVariables.addStringVariable("gcov_gcno_file", gcnoFile.getExecPathString()); @@ -1061,6 +1065,7 @@ public final class CppModel { ruleContext, featureConfiguration, ccToolchain, + fdoSupport, usePicForSharedLibs, // If support is ever added for generating a dwp file for shared // library targets (e.g. when linkstatic=0), then this should change @@ -1108,7 +1113,8 @@ public final class CppModel { } private CppLinkActionBuilder newLinkActionBuilder(Artifact outputArtifact) { - return new CppLinkActionBuilder(ruleContext, outputArtifact, ccToolchain) + return new CppLinkActionBuilder( + ruleContext, outputArtifact, ccToolchain, fdoSupport.getFdoSupport()) .setCrosstoolInputs(ccToolchain.getLink()) .addNonCodeInputs(context.getTransitiveCompilationPrerequisites()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java index 53fa0de33f..c48f7c4cdd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java @@ -24,7 +24,6 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; -import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; @@ -503,7 +502,7 @@ public class FdoSupport { public void configureCompilation(CppCompileActionBuilder builder, CcToolchainFeatures.Variables.Builder buildVariables, RuleContext ruleContext, PathFragment sourceName, PathFragment sourceExecPath, boolean usePic, - FeatureConfiguration featureConfiguration) { + FeatureConfiguration featureConfiguration, FdoSupportProvider fdoSupportProvider) { // It is a bug if this method is called with useLipo if lipo is disabled. However, it is legal // if is is called with !useLipo, even though lipo is enabled. LipoContextProvider lipoInputProvider = CppHelper.getLipoContextProvider(ruleContext); @@ -526,7 +525,7 @@ public class FdoSupport { return; } Iterable<Artifact> auxiliaryInputs = getAuxiliaryInputs( - ruleContext, sourceName, sourceExecPath, usePic, lipoInputProvider); + ruleContext, sourceName, sourceExecPath, usePic, lipoInputProvider, fdoSupportProvider); builder.addMandatoryInputs(auxiliaryInputs); if (!Iterables.isEmpty(auxiliaryInputs)) { if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)) { @@ -551,13 +550,13 @@ public class FdoSupport { */ private Iterable<Artifact> getAuxiliaryInputs( RuleContext ruleContext, PathFragment sourceName, PathFragment sourceExecPath, boolean usePic, - LipoContextProvider lipoContextProvider) { + LipoContextProvider lipoContextProvider, FdoSupportProvider fdoSupportProvider) { // If --fdo_optimize was not specified, we don't have any additional inputs. if (fdoProfile == null) { return ImmutableSet.of(); } else if (fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.LLVM_FDO) { ImmutableSet.Builder<Artifact> auxiliaryInputs = ImmutableSet.builder(); - auxiliaryInputs.add(getFdoSupportProvider(ruleContext).getProfileArtifact()); + auxiliaryInputs.add(fdoSupportProvider.getProfileArtifact()); if (lipoContextProvider != null) { auxiliaryInputs.addAll(getAutoFdoImports(ruleContext, sourceExecPath, lipoContextProvider)); } @@ -570,13 +569,15 @@ public class FdoSupport { Label lipoLabel = ruleContext.getLabel(); auxiliaryInputs.addAll( - getGcdaArtifactsForObjectFileName(ruleContext, objectName, lipoLabel)); + getGcdaArtifactsForObjectFileName( + ruleContext, fdoSupportProvider, objectName, lipoLabel)); if (lipoContextProvider != null) { for (PathFragment importedFile : getImports( getNonLipoObjDir(ruleContext, lipoLabel), objectName)) { if (CppFileTypes.COVERAGE_DATA.matches(importedFile.getBaseName())) { - Artifact gcdaArtifact = getGcdaArtifactsForGcdaPath(ruleContext, importedFile); + Artifact gcdaArtifact = + getGcdaArtifactsForGcdaPath(ruleContext, fdoSupportProvider, importedFile); if (gcdaArtifact == null) { ruleContext.ruleError(String.format( ".gcda file %s is not in the FDO zip (referenced by source file %s)", @@ -605,12 +606,13 @@ public class FdoSupport { * Returns the .gcda file artifacts for a .gcda path from the .gcda.imports file or null if the * referenced .gcda file is not in the FDO zip. */ - private Artifact getGcdaArtifactsForGcdaPath(RuleContext ruleContext, PathFragment gcdaPath) { + private Artifact getGcdaArtifactsForGcdaPath(RuleContext ruleContext, + FdoSupportProvider fdoSupportProvider, PathFragment gcdaPath) { if (!gcdaFiles.contains(gcdaPath)) { return null; } - return getFdoSupportProvider(ruleContext).getGcdaArtifacts().get(gcdaPath); + return fdoSupportProvider.getGcdaArtifacts().get(gcdaPath); } private PathFragment getNonLipoObjDir(RuleContext ruleContext, Label label) { @@ -626,7 +628,7 @@ public class FdoSupport { * symlink to it). */ private ImmutableList<Artifact> getGcdaArtifactsForObjectFileName(RuleContext ruleContext, - PathFragment objectFileName, Label lipoLabel) { + FdoSupportProvider fdoSupportProvider, PathFragment objectFileName, Label lipoLabel) { // We put the .gcda files relative to the location of the .o file in the instrumentation run. String gcdaExt = Iterables.getOnlyElement(CppFileTypes.COVERAGE_DATA.getExtensions()); PathFragment baseName = FileSystemUtils.replaceExtension(objectFileName, gcdaExt); @@ -647,7 +649,7 @@ public class FdoSupport { } } - return ImmutableList.of(getFdoSupportProvider(ruleContext).getGcdaArtifacts().get(gcdaFile)); + return ImmutableList.of(fdoSupportProvider.getGcdaArtifacts().get(gcdaFile)); } @@ -694,13 +696,14 @@ public class FdoSupport { * If AutoFDO is disabled, no build variable is added and returns null. */ @ThreadSafe - public Artifact buildProfileForLtoBackend(FeatureConfiguration featureConfiguration, + public Artifact buildProfileForLtoBackend(FdoSupportProvider fdoSupportProvider, + FeatureConfiguration featureConfiguration, CcToolchainFeatures.Variables.Builder buildVariables, RuleContext ruleContext) { if (!featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)) { return null; } - Artifact profile = getFdoSupportProvider(ruleContext).getProfileArtifact(); + Artifact profile = fdoSupportProvider.getProfileArtifact(); buildVariables.addStringVariable("fdo_profile_path", profile.getExecPathString()); return profile; } @@ -739,11 +742,6 @@ public class FdoSupport { return new FdoSupportProvider(this, profileArtifact, gcdaArtifacts.build()); } - private static FdoSupportProvider getFdoSupportProvider(RuleContext ruleContext) { - return ruleContext.getPrerequisite( - ":cc_toolchain", Mode.TARGET).getProvider(FdoSupportProvider.class); - } - /** * An exception indicating an issue with FDO coverage files. */ 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 8b333b543c..32931bc719 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 @@ -109,6 +109,7 @@ public final class LTOBackendArtifacts { RuleContext ruleContext, FeatureConfiguration featureConfiguration, CcToolchainProvider ccToolchain, + FdoSupportProvider fdoSupport, boolean usePic, boolean generateDwo) { LTOBackendAction.Builder builder = new LTOBackendAction.Builder(); @@ -138,8 +139,8 @@ public final class LTOBackendArtifacts { // The input to the LTO backend step is the bitcode file. buildVariablesBuilder.addStringVariable( "thinlto_input_bitcode_file", bitcodeFile.getExecPath().toString()); - Artifact autoFdoProfile = CppHelper.getFdoSupport(ruleContext).buildProfileForLtoBackend( - featureConfiguration, buildVariablesBuilder, ruleContext); + Artifact autoFdoProfile = fdoSupport.getFdoSupport().buildProfileForLtoBackend( + fdoSupport, featureConfiguration, buildVariablesBuilder, ruleContext); if (autoFdoProfile != null) { builder.addInput(autoFdoProfile); } 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 69b48731f3..2f5b6488d7 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 @@ -657,6 +657,7 @@ public final class LinkCommandLine extends CommandLine { private boolean useTestOnlyFlags; @Nullable private Artifact paramFile; @Nullable private CcToolchainProvider toolchain; + private FdoSupport fdoSupport; private Variables variables; private FeatureConfiguration featureConfiguration; @@ -705,6 +706,10 @@ public final class LinkCommandLine extends CommandLine { featureConfiguration = CcCommon.configureFeatures(ruleContext, ccToolchain); } } + + if (fdoSupport == null) { + fdoSupport = CppHelper.getFdoSupport(ruleContext, ":cc_toolchain").getFdoSupport(); + } } if (variables == null) { @@ -728,7 +733,7 @@ public final class LinkCommandLine extends CommandLine { features, linkstamps, actualLinkstampCompileOptions, - CppHelper.getFdoBuildStamp(ruleContext), + CppHelper.getFdoBuildStamp(ruleContext, fdoSupport), runtimeSolibDir, nativeDeps, useTestOnlyFlags, @@ -746,6 +751,11 @@ public final class LinkCommandLine extends CommandLine { return this; } + public Builder setFdoSupport(FdoSupport fdoSupport) { + this.fdoSupport = fdoSupport; + return this; + } + /** Sets the tool path, with tool being the first thing on the command line */ public Builder setToolPath(String toolPath) { this.toolPath = toolPath; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index 8efef155b9..8abcaef9d8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -220,8 +220,12 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect private CcLibraryHelper initializeCcLibraryHelper(FeatureConfiguration featureConfiguration) { CcLibraryHelper helper = - new CcLibraryHelper(ruleContext, cppSemantics, featureConfiguration, - ccToolchain(ruleContext)); + new CcLibraryHelper( + ruleContext, + cppSemantics, + featureConfiguration, + ccToolchain(ruleContext), + CppHelper.getFdoSupport(ruleContext, ":cc_toolchain")); helper.enableCcSpecificLinkParamsProvider(); helper.enableCcNativeLibrariesProvider(); // TODO(dougk): Configure output artifact with action_config diff --git a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java index 814b18f5f8..ee1bc426ce 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration; import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.rules.cpp.CppLinkAction; import com.google.devtools.build.lib.rules.cpp.CppLinkActionBuilder; +import com.google.devtools.build.lib.rules.cpp.FdoSupport; import com.google.devtools.build.lib.rules.cpp.Link; import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; @@ -197,8 +198,9 @@ public abstract class NativeDepsHelper { } else { sharedLibrary = nativeDeps; } + FdoSupport fdoSupport = CppHelper.getFdoSupport(ruleContext, ":cc_toolchain").getFdoSupport(); CppLinkActionBuilder builder = - new CppLinkActionBuilder(ruleContext, sharedLibrary, configuration, toolchain); + new CppLinkActionBuilder(ruleContext, sharedLibrary, configuration, toolchain, fdoSupport); if (useDynamicRuntime) { builder.setRuntimeInputs(ArtifactCategory.DYNAMIC_LIBRARY, toolchain.getDynamicRuntimeLinkMiddleman(), toolchain.getDynamicRuntimeLinkInputs()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java index d93a499d2f..5c8abce687 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.rules.cpp.CppLinkAction; import com.google.devtools.build.lib.rules.cpp.CppLinkActionBuilder; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; +import com.google.devtools.build.lib.rules.cpp.FdoSupportProvider; import com.google.devtools.build.lib.rules.cpp.IncludeProcessing; import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; @@ -52,6 +53,7 @@ import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag; import com.google.devtools.build.lib.rules.objc.ObjcVariablesExtension.VariableCategory; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; +import javax.annotation.Nullable; /** * Constructs command lines for objc compilation, archiving, and linking. Uses the crosstool @@ -77,7 +79,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport { "preprocess-assemble", "c-compile", "c++-compile"); - private final CcToolchainProvider ccToolchain; + @Nullable private final CcToolchainProvider ccToolchain; + @Nullable private final FdoSupportProvider fdoSupport; /** * Creates a new CompilationSupport instance that uses the c++ rule backend @@ -105,7 +108,16 @@ public class CrosstoolCompilationSupport extends CompilationSupport { IntermediateArtifacts intermediateArtifacts, CompilationAttributes compilationAttributes) { super(ruleContext, buildConfiguration, intermediateArtifacts, compilationAttributes); - this.ccToolchain = CppHelper.getToolchain(ruleContext, ":cc_toolchain"); + // Note some rules like objc_import do not need to compile anything and therefore do not define + // any toolchain attribute. However, they still use the base class to set up other actions like + // the module map action. Here we guard against those cases. + if (ruleContext.attributes().has(":cc_toolchain", BuildType.LABEL)) { + this.ccToolchain = CppHelper.getToolchain(ruleContext, ":cc_toolchain"); + this.fdoSupport = CppHelper.getFdoSupport(ruleContext, ":cc_toolchain"); + } else { + this.ccToolchain = null; + this.fdoSupport = null; + } } @Override @@ -161,7 +173,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport { .build(); CppLinkAction fullyLinkAction = - new CppLinkActionBuilder(ruleContext, outputArchive, ccToolchain) + new CppLinkActionBuilder( + ruleContext, outputArchive, ccToolchain, fdoSupport.getFdoSupport()) .addActionInputs(objcProvider.getObjcLibraries()) .addActionInputs(objcProvider.getCcLibraries()) .addActionInputs(objcProvider.get(IMPORTED_LIBRARY).toSet()) @@ -223,7 +236,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { Artifact binaryToLink = getBinaryToLink(); CppLinkAction executableLinkAction = - new CppLinkActionBuilder(ruleContext, binaryToLink, ccToolchain) + new CppLinkActionBuilder(ruleContext, binaryToLink, ccToolchain, fdoSupport.getFdoSupport()) .setMnemonic("ObjcLink") .addActionInputs(bazelBuiltLibraries) .addActionInputs(objcProvider.getCcLibraries()) @@ -261,7 +274,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport { ruleContext.getFragment(ObjcConfiguration.class)), getFeatureConfiguration(ruleContext), CcLibraryHelper.SourceCategory.CC_AND_OBJC, - ccToolchain) + ccToolchain, + fdoSupport) .addSources(arcSources, ImmutableMap.of("objc_arc", "")) .addSources(nonArcSources, ImmutableMap.of("no_objc_arc", "")) .addSources(privateHdrs) diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index 2157e8673a..2b678f0a47 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -83,7 +83,7 @@ public class CppLinkActionTest extends BuildViewTestCase { }, masterConfig); } - + private final FeatureConfiguration getMockFeatureConfiguration() throws Exception { return CcToolchainFeaturesTest.buildFeatures( CppLinkActionConfigs.getCppLinkActionConfigs( @@ -185,7 +185,7 @@ public class CppLinkActionTest extends BuildViewTestCase { RunfilesProvider runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) .contains("bin _solib_" + cpu + "/libx_Sliba.so"); - + configuredTarget = getConfiguredTarget("//x:b"); linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/b" + extension); assertThat(artifactsToStrings(linkAction.getInputs())).contains("bin x/_objs/b/x/a.pic.o"); @@ -217,7 +217,7 @@ public class CppLinkActionTest extends BuildViewTestCase { .build(); assertThat(linkAction.getEnvironment()).containsEntry("foo", "bar"); } - + /** * This mainly checks that non-static links don't have identical keys. Many options are only * allowed on non-static links, and we test several of them here. @@ -244,7 +244,8 @@ public class CppLinkActionTest extends BuildViewTestCase { new CppLinkActionBuilder( ruleContext, (i & 2) == 0 ? dynamicOutputFile : staticOutputFile, - CppHelper.getToolchain(ruleContext, ":cc_toolchain")) { + CppHelper.getToolchain(ruleContext, ":cc_toolchain"), + CppHelper.getFdoSupport(ruleContext, ":cc_toolchain").getFdoSupport()) { @Override protected Artifact getInterfaceSoBuilder() { return interfaceSoBuilder; @@ -296,7 +297,8 @@ public class CppLinkActionTest extends BuildViewTestCase { new CppLinkActionBuilder( ruleContext, (i & 2) == 0 ? staticOutputFile : dynamicOutputFile, - CppHelper.getToolchain(ruleContext, ":cc_toolchain")) { + CppHelper.getToolchain(ruleContext, ":cc_toolchain"), + CppHelper.getFdoSupport(ruleContext, ":cc_toolchain").getFdoSupport()) { @Override protected Artifact getInterfaceSoBuilder() { return interfaceSoBuilder; @@ -325,7 +327,8 @@ public class CppLinkActionTest extends BuildViewTestCase { RepositoryName.MAIN), ActionsTestUtil.NULL_ARTIFACT_OWNER); CppLinkActionBuilder builder = new CppLinkActionBuilder( - ruleContext, output, CppHelper.getToolchain(ruleContext, ":cc_toolchain")); + ruleContext, output, CppHelper.getToolchain(ruleContext, ":cc_toolchain"), + CppHelper.getFdoSupport(ruleContext, ":cc_toolchain").getFdoSupport()); builder.setLinkType(LinkTargetType.STATIC_LIBRARY); assertTrue(builder.canSplitCommandLine()); @@ -413,7 +416,8 @@ public class CppLinkActionTest extends BuildViewTestCase { getTargetConfiguration() .getBinDirectory(ruleContext.getRule().getRepository())), ruleContext.getConfiguration(), - CppHelper.getToolchain(ruleContext, ":cc_toolchain")) + CppHelper.getToolchain(ruleContext, ":cc_toolchain"), + CppHelper.getFdoSupport(ruleContext, ":cc_toolchain").getFdoSupport()) .addObjectFiles(nonLibraryInputs) .addLibraries(NestedSetBuilder.wrap(Order.LINK_ORDER, libraryInputs)) .setLinkType(type) @@ -425,7 +429,7 @@ public class CppLinkActionTest extends BuildViewTestCase { .setFeatureConfiguration(featureConfiguration); return builder; } - + private CppLinkActionBuilder createLinkBuilder(Link.LinkTargetType type) throws Exception { PathFragment output = new PathFragment("dummyRuleContext/output/path.a"); return createLinkBuilder( |