diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules')
12 files changed, 413 insertions, 193 deletions
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 579bb53bfa..b1639ea9a2 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 @@ -178,7 +178,7 @@ public final class CcCommon { } public TransitiveLipoInfoProvider collectTransitiveLipoLabels(CcCompilationOutputs outputs) { - if (cppConfiguration.getFdoSupport().getFdoRoot() == null + if (CppHelper.getFdoSupport(ruleContext).getFdoRoot() == null || !cppConfiguration.isLipoContextCollector()) { return TransitiveLipoInfoProvider.EMPTY; } 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 79d1c8d2dd..1ec2c57387 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 @@ -922,7 +922,7 @@ public final class CcLibraryHelper { } private TransitiveLipoInfoProvider collectTransitiveLipoInfo(CcCompilationOutputs outputs) { - if (ruleContext.getFragment(CppConfiguration.class).getFdoSupport().getFdoRoot() == null) { + if (CppHelper.getFdoSupport(ruleContext).getFdoRoot() == null) { return TransitiveLipoInfoProvider.EMPTY; } NestedSetBuilder<IncludeScannable> scannableBuilder = NestedSetBuilder.stableOrder(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index 6cc3a5c401..e3efe6c0ce 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -33,15 +33,21 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.License; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException; import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyKey; +import java.io.IOException; import java.util.List; import java.util.Map; @@ -57,6 +63,29 @@ public class CcToolchain implements RuleConfiguredTargetFactory { @Override public ConfiguredTarget create(RuleContext ruleContext) { + CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); + Path fdoZip = ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT + ? cppConfiguration.getFdoZip() + : null; + SkyKey fdoKey = FdoSupportValue.key( + cppConfiguration.getLipoMode(), + fdoZip, + cppConfiguration.getFdoInstrument()); + + SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv(); + FdoSupportValue fdoSupport; + try { + fdoSupport = (FdoSupportValue) skyframeEnv.getValueOrThrow( + fdoKey, FdoException.class, IOException.class); + } catch (FdoException | IOException e) { + ruleContext.ruleError("cannot initialize FDO: " + e.getMessage()); + return null; + } + + if (skyframeEnv.valuesMissing()) { + return null; + } + final Label label = ruleContext.getLabel(); final NestedSet<Artifact> crosstool = ruleContext.getPrerequisite("all_files", Mode.HOST) .getProvider(FileProvider.class).getFilesToBuild(); @@ -72,7 +101,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { final PathFragment runtimeSolibDir = ruleContext.getConfiguration() .getBinFragment().getRelative(runtimeSolibDirBase); - CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); // Static runtime inputs. TransitiveInfoCollection staticRuntimeLibDep = selectDep(ruleContext, "static_runtime_libs", cppConfiguration.getStaticRuntimeLibsLabel()); @@ -173,6 +201,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext) .add(CcToolchainProvider.class, provider) + .add(FdoSupportProvider.class, new FdoSupportProvider(fdoSupport.getFdoSupport())) .setFilesToBuild(new NestedSetBuilder<Artifact>(Order.STABLE_ORDER).build()) .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY)); 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 f151deaf59..c1831ceaf7 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 @@ -25,9 +25,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; -import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.ViewCreationFailedException; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.CompilationMode; @@ -40,7 +38,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters; -import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.util.Preconditions; @@ -51,7 +48,6 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LinkingModeFlags; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; -import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.common.options.OptionsParsingException; import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; @@ -64,7 +60,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.zip.ZipException; /** * This class represents the C/C++ parts of the {@link BuildConfiguration}, @@ -284,7 +279,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { private final boolean toolchainNeedsPic; private final boolean usePicForBinaries; - private final FdoSupport fdoSupport; + private final Path fdoZip; // TODO(bazel-team): All these labels (except for ccCompilerRuleLabel) can be removed once the // transition to the cc_compiler rule is complete. @@ -390,10 +385,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } } - this.fdoSupport = new FdoSupport( - cppOptions.fdoInstrument, params.fdoZip, - cppOptions.lipoMode, execRoot); - + this.fdoZip = params.fdoZip; this.stripBinaries = (cppOptions.stripBinaries == StripMode.ALWAYS || (cppOptions.stripBinaries == StripMode.SOMETIMES @@ -1481,8 +1473,9 @@ public class CppConfiguration extends BuildConfiguration.Fragment { * Returns true if it is AutoFDO LIPO build. */ public boolean isAutoFdoLipo() { - return cppOptions.fdoOptimize != null && FdoSupport.isAutoFdo(cppOptions.fdoOptimize) - && getLipoMode() != LipoMode.OFF; + return cppOptions.fdoOptimize != null + && CppFileTypes.GCC_AUTO_PROFILE.matches(cppOptions.fdoOptimize) + && getLipoMode() != LipoMode.OFF; } /** @@ -1639,13 +1632,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } /** - * Returns the FDO support object. - */ - public FdoSupport getFdoSupport() { - return fdoSupport; - } - - /** * Return the name of the directory (relative to the bin directory) that * holds mangled links to shared libraries. This name is always set to * the '{@code _solib_<cpu_archictecture_name>}. @@ -1892,35 +1878,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } @Override - public void prepareHook(Path execRoot) throws ViewCreationFailedException { - try { - getFdoSupport().prepareToBuild(execRoot); - } catch (ZipException e) { - throw new ViewCreationFailedException("Error reading provided FDO zip file", e); - } catch (FdoException | IOException e) { - throw new ViewCreationFailedException("Error while initializing FDO support", e); - } - } - - @Override - public void declareSkyframeDependencies(Environment env) { - getFdoSupport().declareSkyframeDependencies(env, execRoot); - } - - @Override - public void addRoots(List<Root> roots) { - // Fdo root can only exist for the target configuration. - FdoSupport fdoSupport = getFdoSupport(); - if (fdoSupport.getFdoRoot() != null) { - roots.add(fdoSupport.getFdoRoot()); - } - } - - @Override public Map<String, String> getCoverageEnvironment() { ImmutableMap.Builder<String, String> env = ImmutableMap.builder(); env.put("COVERAGE_GCOV_PATH", getGcovExecutable().getPathString()); - PathFragment fdoInstrument = getFdoSupport().getFdoInstrument(); + PathFragment fdoInstrument = cppOptions.fdoInstrument; if (fdoInstrument != null) { env.put("FDO_DIR", fdoInstrument.getPathString()); } @@ -1990,6 +1951,14 @@ public class CppConfiguration extends BuildConfiguration.Fragment { ); } + public PathFragment getFdoInstrument() { + return cppOptions.fdoInstrument; + } + + public Path getFdoZip() { + return fdoZip; + } + /** * Return set of features enabled by the CppConfiguration, specifically * the FDO and LIPO related features enabled by options. @@ -1997,15 +1966,15 @@ public class CppConfiguration extends BuildConfiguration.Fragment { @Override public ImmutableSet<String> configurationEnabledFeatures(RuleContext ruleContext) { ImmutableSet.Builder<String> requestedFeatures = ImmutableSet.builder(); - FdoSupport fdoSupport = getFdoSupport(); - if (fdoSupport.getFdoInstrument() != null) { + if (cppOptions.fdoInstrument != null) { requestedFeatures.add(CppRuleClasses.FDO_INSTRUMENT); } - if (fdoSupport.getFdoOptimizeProfile() != null - && !fdoSupport.isAutoFdoEnabled()) { + + boolean isFdo = fdoZip != null && compilationMode == CompilationMode.OPT; + if (isFdo && !CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) { requestedFeatures.add(CppRuleClasses.FDO_OPTIMIZE); } - if (fdoSupport.isAutoFdoEnabled()) { + if (isFdo && CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) { requestedFeatures.add(CppRuleClasses.AUTOFDO); } if (isLipoOptimizationOrInstrumentation()) { 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 97db3a514f..300c87e122 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 @@ -214,6 +214,13 @@ public class CppHelper { return false; } + @Nullable public static FdoSupport getFdoSupport(RuleContext ruleContext) { + return ruleContext + .getPrerequisite(":cc_toolchain", Mode.TARGET) + .getProvider(FdoSupportProvider.class) + .getFdoSupport(); + } + /** * This almost trivial method looks up the :cc_toolchain attribute on the rule context, makes sure * that it refers to a rule that has a {@link CcToolchainProvider} (gives an error otherwise), and @@ -508,8 +515,9 @@ public class CppHelper { /** * Returns the FDO build subtype. */ - public static String getFdoBuildStamp(CppConfiguration cppConfiguration) { - if (cppConfiguration.getFdoSupport().isAutoFdoEnabled()) { + public static String getFdoBuildStamp(RuleContext ruleContext) { + CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); + if (getFdoSupport(ruleContext).isAutoFdoEnabled()) { return (cppConfiguration.getLipoMode() == LipoMode.BINARY) ? "ALIPO" : "AFDO"; } if (cppConfiguration.isFdo()) { 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 95d6379f18..b0a1b7d825 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 @@ -1213,24 +1213,6 @@ public final class CppLinkAction extends AbstractAction { this.runtimeSolibDir = runtimeSolibDir; return this; } - - /** - * Creates a builder without the need for a {@link RuleContext}. - * This is to be used exclusively for testing purposes. - * - * <p>Link stamping is not supported if using this method. - */ - @VisibleForTesting - public static Builder createTestBuilder( - final ActionOwner owner, final AnalysisEnvironment analysisEnvironment, - final Artifact output, BuildConfiguration config) { - return new Builder(null, output, config, analysisEnvironment, null) { - @Override - protected ActionOwner getOwner() { - return owner; - } - }; - } } /** 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 c8a8c1a19b..bd8bbfa1a2 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 @@ -287,7 +287,7 @@ public final class CppModel { } builder.setExtraSystemIncludePrefixes(additionalIncludes); - builder.setFdoBuildStamp(CppHelper.getFdoBuildStamp(cppConfiguration)); + builder.setFdoBuildStamp(CppHelper.getFdoBuildStamp(ruleContext)); builder.setFeatureConfiguration(featureConfiguration); return builder; } @@ -367,8 +367,8 @@ public final class CppModel { } if (ccRelativeName != null) { - cppConfiguration.getFdoSupport().configureCompilation(builder, buildVariables, ruleContext, - ccRelativeName, autoFdoImportPath, usePic, featureConfiguration); + CppHelper.getFdoSupport(ruleContext).configureCompilation(builder, buildVariables, + ruleContext, ccRelativeName, autoFdoImportPath, usePic, featureConfiguration); } if (gcnoFile != null) { buildVariables.addVariable("gcov_gcno_file", gcnoFile.getExecPathString()); 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 b182c8cf04..18f2cf18df 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 @@ -25,7 +25,7 @@ import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.skyframe.FileValue; @@ -47,20 +47,13 @@ import java.util.zip.ZipException; * Support class for FDO (feedback directed optimization) and LIPO (lightweight inter-procedural * optimization). * - * <p>There is a 1:1 relationship between {@link CppConfiguration} objects and {@code FdoSupport} - * objects. The FDO support of a build configuration can be retrieved using {@link - * CppConfiguration#getFdoSupport()}. - * - * <p>With respect to thread-safety, the {@link #prepareToBuild} method is not thread-safe, and must - * not be called concurrently with other methods on this class. - * * <p>Here follows a quick run-down of how FDO/LIPO builds work (for non-FDO/LIPO builds, none * of this applies): * - * <p>{@link CppConfiguration#prepareHook} is called before the analysis phase, which calls - * {@link #prepareToBuild}, which extracts the FDO .zip (in case we work with an explicitly - * generated FDO profile file) or analyzes the .afdo.imports file next to the .afdo file (if - * AutoFDO is in effect). + * <p>{@link FdoSupport#create} is called from {@link FdoSupportFunction} (a {@link SkyFunction}), + * which is requested from Skyframe by the {@code cc_toolchain} rule. It extracts the FDO .zip (in + * case we work with an explicitly generated FDO profile file) or analyzes the .afdo.imports file + * next to the .afdo file (if AutoFDO is in effect). * * <p>.afdo.imports files contain one import a line. A line is two paths separated by a colon, * with functions in the second path being referenced by functions in the first path. These are @@ -108,9 +101,30 @@ import java.util.zip.ZipException; * {@code CachingAnalysisEnvironment.allowRegisteringActions}, which causes actions to be silently * discarded after configured targets are created. */ +@Immutable public class FdoSupport { /** + * The FDO mode we are operating in. + * + * LIPO can only be active if this is not <code>OFF</code>, but all of the modes below can work + * with LIPO either off or on. + */ + private enum FdoMode { + /** FDO is turned off. */ + OFF, + + /** Profiling-based FDO using an explicitly recorded profile. */ + VANILLA, + + /** FDO based on automatically collected data. */ + AUTO_FDO, + + /** Instrumentation-based FDO implemented on LLVM. */ + LLVM_FDO, + } + + /** * Path within profile data .zip files that is considered the root of the * profile information directory tree. */ @@ -170,24 +184,15 @@ public class FdoSupport { private final LipoMode lipoMode; /** - * Flag indicating whether to use AutoFDO (as opposed to - * instrumentation-based FDO). + * FDO mode. */ - private final boolean useAutoFdo; - - /** - * Flag indicating whether to use LLVM instrumentation-based FDO (as - * opposed to GCC instrumentation-based FDO). - */ - private final boolean useLLVMFdo; + private final FdoMode fdoMode; /** * The {@code .gcda} files that have been extracted from the ZIP file, * relative to the root of the ZIP file. - * - * <p>Set only in {@link #prepareToBuild}. */ - private ImmutableSet<PathFragment> gcdaFiles = ImmutableSet.of(); + private final ImmutableSet<PathFragment> gcdaFiles; /** * Multimap from .gcda file base names to auxiliary input files. @@ -198,10 +203,8 @@ public class FdoSupport { * * <p>The contents of the multimap are copied verbatim from the .gcda.imports * files and not yet checked for validity. - * - * <p>Set only in {@link #prepareToBuild}. */ - private ImmutableMultimap<PathFragment, PathFragment> imports; + private final ImmutableMultimap<PathFragment, PathFragment> imports; /** * Creates an FDO support object. @@ -210,83 +213,122 @@ public class FdoSupport { * @param fdoProfile path to the profile file passed to --fdo_optimize option * @param lipoMode value of the --lipo_mode option */ - public FdoSupport(PathFragment fdoInstrument, Path fdoProfile, LipoMode lipoMode, Path execRoot) { + private FdoSupport(FdoMode fdoMode, LipoMode lipoMode, Root fdoRoot, PathFragment fdoRootExecPath, + PathFragment fdoInstrument, Path fdoProfile, FdoZipContents fdoZipContents) { this.fdoInstrument = fdoInstrument; this.fdoProfile = fdoProfile; - this.fdoRoot = (fdoProfile == null) - ? null - : Root.asDerivedRoot(execRoot, execRoot.getRelative("blaze-fdo")); - this.fdoRootExecPath = fdoProfile == null - ? null - : fdoRoot.getExecPath().getRelative(FileSystemUtils.removeExtension( - new PathFragment("_fdo").getChild(fdoProfile.getBaseName()))); + this.fdoRoot = fdoRoot; + this.fdoRootExecPath = fdoRootExecPath; this.fdoPath = fdoProfile == null ? null : FileSystemUtils.removeExtension(new PathFragment("_fdo").getChild( fdoProfile.getBaseName())); this.lipoMode = lipoMode; - this.useAutoFdo = fdoProfile != null && isAutoFdo(fdoProfile.getBaseName()); - this.useLLVMFdo = fdoProfile != null && isLLVMFdo(fdoProfile.getBaseName()); + this.fdoMode = fdoMode; + this.gcdaFiles = fdoZipContents.gcdaFiles; + this.imports = fdoZipContents.imports; } public Root getFdoRoot() { return fdoRoot; } - public void declareSkyframeDependencies(SkyFunction.Environment env, Path execRoot) { + /** + * Creates an initialized {@link FdoSupport} instance. + */ + static FdoSupport create(SkyFunction.Environment env, PathFragment fdoInstrument, + Path fdoProfile, LipoMode lipoMode, Path execRoot) throws IOException, FdoException { + FdoMode fdoMode; + if (fdoProfile != null && isAutoFdo(fdoProfile.getBaseName())) { + fdoMode = FdoMode.AUTO_FDO; + } else if (fdoProfile != null && isLLVMFdo(fdoProfile.getBaseName())) { + fdoMode = FdoMode.LLVM_FDO; + } else if (fdoProfile != null) { + fdoMode = FdoMode.VANILLA; + } else { + fdoMode = FdoMode.OFF; + } + + if (fdoProfile == null) { + lipoMode = LipoMode.OFF; + } + + Root fdoRoot = (fdoProfile == null) + ? null + : Root.asDerivedRoot(execRoot, execRoot.getRelative("blaze-fdo")); + + PathFragment fdoRootExecPath = fdoProfile == null + ? null + : fdoRoot.getExecPath().getRelative(FileSystemUtils.removeExtension( + new PathFragment("_fdo").getChild(fdoProfile.getBaseName()))); + if (fdoProfile != null) { - if (isLipoEnabled()) { + if (lipoMode != LipoMode.OFF) { // Incrementality is not supported for LIPO builds, see FdoSupport#scannables. // Ensure that the Skyframe value containing the configuration will not be reused to avoid // incrementality issues. PrecomputedValue.dependOnBuildId(env); - return; - } - - // IMPORTANT: Keep the following in sync with #prepareToBuild. - Path path; - if (useAutoFdo) { - path = fdoProfile.getParentDirectory().getRelative( - fdoProfile.getBaseName() + ".imports"); } else { - path = fdoProfile; + Path path = fdoMode == FdoMode.AUTO_FDO ? getAutoFdoImportsPath(fdoProfile) : fdoProfile; + env.getValue(FileValue.key(RootedPath.toRootedPathMaybeUnderRoot(path, + ImmutableList.of(execRoot)))); } - env.getValue(FileValue.key(RootedPath.toRootedPathMaybeUnderRoot(path, - ImmutableList.of(execRoot)))); + } + + if (env.valuesMissing()) { + return null; + } + + FdoZipContents fdoZipContents = extractFdoZip( + fdoMode, lipoMode, execRoot, fdoProfile, fdoRootExecPath); + return new FdoSupport( + fdoMode, lipoMode, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, fdoZipContents); + } + + @Immutable + private static class FdoZipContents { + private final ImmutableSet<PathFragment> gcdaFiles; + private final ImmutableMultimap<PathFragment, PathFragment> imports; + + private FdoZipContents(ImmutableSet<PathFragment> gcdaFiles, + ImmutableMultimap<PathFragment, PathFragment> imports) { + this.gcdaFiles = gcdaFiles; + this.imports = imports; } } /** - * Prepares the FDO support for building. + * Extracts the FDO zip file and collects data from it that's needed during analysis. * * <p>When an {@code --fdo_optimize} compile is requested, unpacks the given * FDO gcda zip file into a clean working directory under execRoot. * * @throws FdoException if the FDO ZIP contains a file of unknown type */ - @ThreadHostile // must be called before starting the build - public void prepareToBuild(Path execRoot) + private static FdoZipContents extractFdoZip(FdoMode fdoMode, LipoMode lipoMode, Path execRoot, + Path fdoProfile, PathFragment fdoRootExecPath) throws IOException, FdoException { // The execRoot != null case is only there for testing. We cannot provide a real ZIP file in // tests because ZipFileSystem does not work with a ZIP on an in-memory file system. // IMPORTANT: Keep in sync with #declareSkyframeDependencies to avoid incrementality issues. + ImmutableSet<PathFragment> gcdaFiles = ImmutableSet.of(); + ImmutableMultimap<PathFragment, PathFragment> imports = ImmutableMultimap.of(); + if (fdoProfile != null && execRoot != null) { Path fdoDirPath = execRoot.getRelative(fdoRootExecPath); FileSystemUtils.deleteTreesBelow(fdoDirPath); FileSystemUtils.createDirectoryAndParents(fdoDirPath); - if (useAutoFdo) { - Path fdoImports = fdoProfile.getParentDirectory().getRelative( - fdoProfile.getBaseName() + ".imports"); - if (isLipoEnabled()) { - imports = readAutoFdoImports(fdoImports); + if (fdoMode == FdoMode.AUTO_FDO) { + if (lipoMode != LipoMode.OFF) { + imports = readAutoFdoImports(getAutoFdoImportsPath(fdoProfile)); } FileSystemUtils.ensureSymbolicLink( - execRoot.getRelative(getAutoProfilePath()), fdoProfile); - } else if (useLLVMFdo) { + execRoot.getRelative(getAutoProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile); + } else if (fdoMode == FdoMode.LLVM_FDO) { FileSystemUtils.ensureSymbolicLink( - execRoot.getRelative(getLLVMProfilePath()), fdoProfile); + execRoot.getRelative(getLLVMProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile); } else { Path zipFilePath = new ZipFileSystem(fdoProfile).getRootDirectory(); if (!zipFilePath.getRelative("blaze-out").isDirectory()) { @@ -296,11 +338,13 @@ public class FdoSupport { ImmutableSet.Builder<PathFragment> gcdaFilesBuilder = ImmutableSet.builder(); ImmutableMultimap.Builder<PathFragment, PathFragment> importsBuilder = ImmutableMultimap.builder(); - extractFdoZip(zipFilePath, fdoDirPath, gcdaFilesBuilder, importsBuilder); + extractFdoZipDirectory(zipFilePath, fdoDirPath, gcdaFilesBuilder, importsBuilder); gcdaFiles = gcdaFilesBuilder.build(); imports = importsBuilder.build(); } } + + return new FdoZipContents(gcdaFiles, imports); } /** @@ -320,7 +364,7 @@ public class FdoSupport { * @throws IOException if any of the I/O operations failed * @throws FdoException if the FDO ZIP contains a file of unknown type */ - private void extractFdoZip(Path sourceDir, Path targetDir, + private static void extractFdoZipDirectory(Path sourceDir, Path targetDir, ImmutableSet.Builder<PathFragment> gcdaFilesBuilder, ImmutableMultimap.Builder<PathFragment, PathFragment> importsBuilder) throws IOException, FdoException { @@ -328,7 +372,7 @@ public class FdoSupport { Path targetFile = targetDir.getRelative(sourceFile.getBaseName()); if (sourceFile.isDirectory()) { targetFile.createDirectory(); - extractFdoZip(sourceFile, targetFile, gcdaFilesBuilder, importsBuilder); + extractFdoZipDirectory(sourceFile, targetFile, gcdaFilesBuilder, importsBuilder); } else { if (CppFileTypes.COVERAGE_DATA.matches(sourceFile)) { FileSystemUtils.copyFile(sourceFile, targetFile); @@ -343,12 +387,13 @@ public class FdoSupport { } } + /** * Reads a .gcda.imports file and stores the imports information. * * @throws FdoException if an auxiliary LIPO input was not found */ - private void readCoverageImports(Path importsFile, + private static void readCoverageImports(Path importsFile, ImmutableMultimap.Builder<PathFragment, PathFragment> importsBuilder) throws IOException, FdoException { PathFragment key = importsFile.asFragment().relativeTo(ZIP_ROOT); @@ -373,7 +418,7 @@ public class FdoSupport { /** * Reads a .afdo.imports file and stores the imports information. */ - private ImmutableMultimap<PathFragment, PathFragment> readAutoFdoImports(Path importsFile) + private static ImmutableMultimap<PathFragment, PathFragment> readAutoFdoImports(Path importsFile) throws IOException, FdoException { ImmutableMultimap.Builder<PathFragment, PathFragment> importBuilder = ImmutableMultimap.builder(); @@ -396,6 +441,10 @@ public class FdoSupport { return importBuilder.build(); } + private static Path getAutoFdoImportsPath(Path fdoProfile) { + return fdoProfile.getParentDirectory().getRelative(fdoProfile.getBaseName() + ".imports"); + } + /** * Returns the imports from the .afdo.imports file of a source file. * @@ -403,7 +452,7 @@ public class FdoSupport { */ private Collection<Artifact> getAutoFdoImports(RuleContext ruleContext, PathFragment sourceExecPath, LipoContextProvider lipoContextProvider) { - Preconditions.checkState(isLipoEnabled()); + Preconditions.checkState(lipoMode != LipoMode.OFF); ImmutableCollection<PathFragment> afdoImports = imports.get(sourceExecPath); Preconditions.checkState(afdoImports != null, "AutoFDO import data missing for %s", sourceExecPath); @@ -427,7 +476,7 @@ public class FdoSupport { * @param objectName the object file */ private Iterable<PathFragment> getImports(PathFragment objDirectory, PathFragment objectName) { - Preconditions.checkState(isLipoEnabled()); + Preconditions.checkState(lipoMode != LipoMode.OFF); Preconditions.checkState(imports != null, "Tried to look up imports of uninitialized FDOSupport"); PathFragment key = objDirectory.getRelative(FileSystemUtils.removeExtension(objectName)); @@ -449,7 +498,7 @@ public class FdoSupport { // 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); - Preconditions.checkArgument(lipoInputProvider == null || isLipoEnabled()); + Preconditions.checkArgument(lipoInputProvider == null || lipoMode != LipoMode.OFF); // FDO is disabled -> do nothing. if ((fdoInstrument == null) && (fdoRoot == null)) { @@ -474,12 +523,12 @@ public class FdoSupport { if (!Iterables.isEmpty(auxiliaryInputs)) { if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)) { buildVariables.addVariable("fdo_profile_path", - getAutoProfilePath().getPathString()); + getAutoProfilePath(fdoProfile, fdoRootExecPath).getPathString()); } if (featureConfiguration.isEnabled(CppRuleClasses.FDO_OPTIMIZE)) { - if (useLLVMFdo) { + if (fdoMode == FdoMode.LLVM_FDO) { buildVariables.addVariable("fdo_profile_path", - getLLVMProfilePath().getPathString()); + getLLVMProfilePath(fdoProfile, fdoRootExecPath).getPathString()); } else { buildVariables.addVariable("fdo_profile_path", fdoRootExecPath.getPathString()); @@ -498,12 +547,12 @@ public class FdoSupport { // If --fdo_optimize was not specified, we don't have any additional inputs. if (fdoProfile == null) { return ImmutableSet.of(); - } else if (useAutoFdo || useLLVMFdo) { + } else if (fdoMode == FdoMode.AUTO_FDO || fdoMode == FdoMode.LLVM_FDO) { ImmutableSet.Builder<Artifact> auxiliaryInputs = ImmutableSet.builder(); - PathFragment profileRootRelativePath = (useLLVMFdo) - ? getLLVMProfileRootRelativePath() - : getAutoProfileRootRelativePath(); + PathFragment profileRootRelativePath = fdoMode == FdoMode.LLVM_FDO + ? getLLVMProfileRootRelativePath(fdoProfile) + : getAutoProfileRootRelativePath(fdoProfile); Artifact artifact = env.getDerivedArtifact( fdoPath.getRelative(profileRootRelativePath), fdoRoot); env.registerAction(new FdoStubAction(ruleContext.getActionOwner(), artifact)); @@ -607,37 +656,29 @@ public class FdoSupport { } - private PathFragment getAutoProfilePath() { - return fdoRootExecPath.getRelative(getAutoProfileRootRelativePath()); + private static PathFragment getAutoProfilePath(Path fdoProfile, PathFragment fdoRootExecPath) { + return fdoRootExecPath.getRelative(getAutoProfileRootRelativePath(fdoProfile)); } - private PathFragment getAutoProfileRootRelativePath() { + private static PathFragment getAutoProfileRootRelativePath(Path fdoProfile) { return new PathFragment(fdoProfile.getBaseName()); } - private PathFragment getLLVMProfilePath() { - return fdoRootExecPath.getRelative(getLLVMProfileRootRelativePath()); + private static PathFragment getLLVMProfilePath(Path fdoProfile, PathFragment fdoRootExecPath) { + return fdoRootExecPath.getRelative(getLLVMProfileRootRelativePath(fdoProfile)); } - private PathFragment getLLVMProfileRootRelativePath() { + private static PathFragment getLLVMProfileRootRelativePath(Path fdoProfile) { return new PathFragment(fdoProfile.getBaseName()); } /** - * Returns whether LIPO is enabled. - */ - @ThreadSafe - public boolean isLipoEnabled() { - return fdoProfile != null && lipoMode != LipoMode.OFF; - } - - /** * Returns whether AutoFDO is enabled. */ @ThreadSafe public boolean isAutoFdoEnabled() { - return useAutoFdo; + return fdoMode == FdoMode.AUTO_FDO; } /** @@ -664,29 +705,6 @@ public class FdoSupport { } /** - * Returns the path of the FDO zip containing the .gcda profile files, or null - * if FDO is not enabled. - */ - @VisibleForTesting - public Path getFdoOptimizeProfile() { - return fdoProfile; - } - - /** - * Returns the path fragment of the instrumentation output dir for gcc when - * FDO is enabled, or null if FDO is not enabled. - */ - @ThreadSafe - public PathFragment getFdoInstrument() { - return fdoInstrument; - } - - @VisibleForTesting - public void setGcdaFilesForTesting(ImmutableSet<PathFragment> gcdaFiles) { - this.gcdaFiles = gcdaFiles; - } - - /** * An exception indicating an issue with FDO coverage files. */ public static final class FdoException extends Exception { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java new file mode 100644 index 0000000000..b232a31582 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java @@ -0,0 +1,76 @@ +// Copyright 2014 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.cpp; + +import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionException.Transience; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +import java.io.IOException; + +import javax.annotation.Nullable; + +/** + * Wrapper for {@link FdoSupport} that turns it into a {@link SkyFunction}. + */ +public class FdoSupportFunction implements SkyFunction { + + /** + * Wrapper for FDO exceptions. + */ + public class FdoSkyException extends SkyFunctionException { + public FdoSkyException(Exception cause, Transience transience) { + super(cause, transience); + } + } + + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws FdoSkyException, InterruptedException { + BlazeDirectories blazeDirectories = PrecomputedValue.BLAZE_DIRECTORIES.get(env); + if (env.valuesMissing()) { + return null; + } + + Path execRoot = blazeDirectories.getExecRoot(); + FdoSupportValue.Key key = (FdoSupportValue.Key) skyKey.argument(); + FdoSupport fdoSupport; + try { + fdoSupport = FdoSupport.create(env, + key.getFdoInstrument(), key.getFdoZip(), key.getLipoMode(), execRoot); + if (env.valuesMissing()) { + return null; + } + } catch (FdoException e) { + throw new FdoSkyException(e, Transience.PERSISTENT); + } catch (IOException e) { + throw new FdoSkyException(e, Transience.TRANSIENT); + } + + return new FdoSupportValue(fdoSupport); + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportProvider.java new file mode 100644 index 0000000000..f97621421a --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportProvider.java @@ -0,0 +1,34 @@ +// Copyright 2014 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.cpp; + +import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; + +/** + * A {@link TransitiveInfoProvider} so that {@code cc_toolchain} can pass {@link FdoSupport} to the + * C++ rules. + */ +@Immutable +public class FdoSupportProvider implements TransitiveInfoProvider { + private final FdoSupport fdoSupport; + + public FdoSupportProvider(FdoSupport fdoSupport) { + this.fdoSupport = fdoSupport; + } + + public FdoSupport getFdoSupport() { + return fdoSupport; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java new file mode 100644 index 0000000000..eb03db6a3f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java @@ -0,0 +1,101 @@ +// Copyright 2014 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.cpp; + +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +import java.util.Objects; + +/** + * Wrapper for {@link FdoSupport}. + * + * <p>Things could probably be refactored such the attributes of {@link FdoSupport} are moved here + * and the code in it to {@link FdoSupportFunction}. This would let us eliminate {@link FdoSupport}. + * + * <p>The eventual plan is to migrate FDO functionality to the execution phase once directory + * artifacts work better, so this may not be worth it. + */ +@Immutable +public class FdoSupportValue implements SkyValue { + public static final SkyFunctionName SKYFUNCTION = SkyFunctionName.create("FDO_SUPPORT"); + + /** + * {@link SkyKey} for {@link FdoSupportValue}. + */ + @Immutable + public static class Key { + private final LipoMode lipoMode; + private final Path fdoZip; + private final PathFragment fdoInstrument; + + private Key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument) { + this.lipoMode = lipoMode; + this.fdoZip = fdoZip; + this.fdoInstrument = fdoInstrument; + } + + public LipoMode getLipoMode() { + return lipoMode; + } + + public Path getFdoZip() { + return fdoZip; + } + + public PathFragment getFdoInstrument() { + return fdoInstrument; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + + if (!(o instanceof Key)) { + return false; + } + + Key that = (Key) o; + return Objects.equals(this.lipoMode, that.lipoMode) + && Objects.equals(this.fdoZip, that.fdoZip) + && Objects.equals(this.fdoInstrument, that.fdoInstrument); + } + + @Override + public int hashCode() { + return Objects.hash(lipoMode, fdoZip, fdoInstrument); + } + } + + private final FdoSupport fdoSupport; + + FdoSupportValue(FdoSupport fdoSupport) { + this.fdoSupport = fdoSupport; + } + + public FdoSupport getFdoSupport() { + return fdoSupport; + } + + public static SkyKey key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument) { + return SkyKey.create(SKYFUNCTION, new Key(lipoMode, fdoZip, fdoInstrument)); + } +} 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 98ed76fa10..79c24e6bef 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 @@ -75,6 +75,7 @@ public final class LinkCommandLine extends CommandLine { private final ImmutableSet<String> features; private final ImmutableMap<Artifact, Artifact> linkstamps; private final ImmutableList<String> linkstampCompileOptions; + @Nullable private final String fdoBuildStamp; @Nullable private final PathFragment runtimeSolibDir; private final boolean nativeDeps; private final boolean useTestOnlyFlags; @@ -84,6 +85,7 @@ public final class LinkCommandLine extends CommandLine { @Nullable private final Artifact paramFile; @Nullable private final Artifact interfaceSoBuilder; + /** * A string constant for the c++ link action, used to access the feature * configuration. @@ -105,6 +107,7 @@ public final class LinkCommandLine extends CommandLine { ImmutableSet<String> features, ImmutableMap<Artifact, Artifact> linkstamps, ImmutableList<String> linkstampCompileOptions, + @Nullable String fdoBuildStamp, @Nullable PathFragment runtimeSolibDir, boolean nativeDeps, boolean useTestOnlyFlags, @@ -161,6 +164,7 @@ public final class LinkCommandLine extends CommandLine { this.features = Preconditions.checkNotNull(features); this.linkstamps = Preconditions.checkNotNull(linkstamps); this.linkstampCompileOptions = linkstampCompileOptions; + this.fdoBuildStamp = fdoBuildStamp; this.runtimeSolibDir = runtimeSolibDir; this.nativeDeps = nativeDeps; this.useTestOnlyFlags = useTestOnlyFlags; @@ -590,7 +594,6 @@ public final class LinkCommandLine extends CommandLine { } // Stamp FDO builds with FDO subtype string - String fdoBuildStamp = CppHelper.getFdoBuildStamp(cppConfiguration); if (fdoBuildStamp != null) { optionList.add("-D" + CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\""); } @@ -1052,8 +1055,7 @@ public final class LinkCommandLine extends CommandLine { } CcToolchainFeatures.Variables.Builder buildVariables = new CcToolchainFeatures.Variables.Builder(); - CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); - cppConfiguration.getFdoSupport().getLinkOptions(featureConfiguration, buildVariables); + CppHelper.getFdoSupport(ruleContext).getLinkOptions(featureConfiguration, buildVariables); variables = buildVariables.build(); } return new LinkCommandLine( @@ -1071,6 +1073,7 @@ public final class LinkCommandLine extends CommandLine { features, linkstamps, actualLinkstampCompileOptions, + CppHelper.getFdoBuildStamp(ruleContext), runtimeSolibDir, nativeDeps, useTestOnlyFlags, |