diff options
Diffstat (limited to 'src')
9 files changed, 98 insertions, 68 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 8d7979ba7c..2be679614c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -177,7 +177,8 @@ public final class RuleContext extends TargetContext private final ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap; private final ImmutableMap<Label, ConfigMatchingProvider> configConditions; private final AspectAwareAttributeMapper attributes; - private final ImmutableSet<String> features; + private final ImmutableSet<String> enabledFeatures; + private final ImmutableSet<String> disabledFeatures; private final String ruleClassNameForLogging; private final BuildConfiguration hostConfiguration; private final PatchTransition disableLipoTransition; @@ -217,7 +218,11 @@ public final class RuleContext extends TargetContext this.filesetEntryMap = filesetEntryMap; this.configConditions = configConditions; this.attributes = new AspectAwareAttributeMapper(attributes, aspectAttributes); - this.features = getEnabledFeatures(); + Set<String> allEnabledFeatures = new HashSet<>(); + Set<String> allDisabledFeatures = new HashSet<>(); + getAllFeatures(allEnabledFeatures, allDisabledFeatures); + this.enabledFeatures = ImmutableSortedSet.copyOf(allEnabledFeatures); + this.disabledFeatures = ImmutableSortedSet.copyOf(allDisabledFeatures); this.ruleClassNameForLogging = ruleClassNameForLogging; this.hostConfiguration = builder.hostConfiguration; this.disableLipoTransition = builder.disableLipoTransition; @@ -225,7 +230,7 @@ public final class RuleContext extends TargetContext this.toolchainContext = toolchainContext; } - private ImmutableSet<String> getEnabledFeatures() { + private void getAllFeatures(Set<String> allEnabledFeatures, Set<String> allDisabledFeatures) { Set<String> globallyEnabled = new HashSet<>(); Set<String> globallyDisabled = new HashSet<>(); parseFeatures(getConfiguration().getDefaultFeatures(), globallyEnabled, globallyDisabled); @@ -237,23 +242,26 @@ public final class RuleContext extends TargetContext if (attributes().has("features", Type.STRING_LIST)) { parseFeatures(attributes().get("features", Type.STRING_LIST), ruleEnabled, ruleDisabled); } + Set<String> ruleDisabledFeatures = Sets.union(ruleDisabled, Sets.difference(packageDisabled, ruleEnabled)); - Set<String> disabledFeatures = Sets.union(ruleDisabledFeatures, globallyDisabled); + allDisabledFeatures.addAll(Sets.union(ruleDisabledFeatures, globallyDisabled)); for (ImmutableMap.Entry<Class<? extends Fragment>, Fragment> entry : getConfiguration().getAllFragments().entrySet()) { if (isLegalFragment(entry.getKey())) { globallyEnabled.addAll( entry .getValue() - .configurationEnabledFeatures(this, ImmutableSortedSet.copyOf(disabledFeatures))); + .configurationEnabledFeatures( + this, ImmutableSortedSet.copyOf(allDisabledFeatures))); } } + Set<String> packageFeatures = Sets.difference(Sets.union(globallyEnabled, packageEnabled), packageDisabled); Set<String> ruleFeatures = Sets.difference(Sets.union(packageFeatures, ruleEnabled), ruleDisabled); - return ImmutableSortedSet.copyOf(Sets.difference(ruleFeatures, globallyDisabled)); + allEnabledFeatures.addAll(Sets.difference(ruleFeatures, globallyDisabled)); } private void parseFeatures(Iterable<String> features, Set<String> enabled, Set<String> disabled) { @@ -1371,7 +1379,12 @@ public final class RuleContext extends TargetContext * @return the set of features applicable for the current rule's package. */ public ImmutableSet<String> getFeatures() { - return features; + return enabledFeatures; + } + + /** @return the set of features that are disabled for the current rule's package. */ + public ImmutableSet<String> getDisabledFeatures() { + return disabledFeatures; } @Override 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 69c3f6061d..a0629dab46 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 @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfig import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoMode; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -698,6 +699,7 @@ public final class CcCommon { } ImmutableSet<String> allUnsupportedFeatures = unsupportedFeaturesBuilder.build(); ImmutableSet.Builder<String> allRequestedFeaturesBuilder = ImmutableSet.builder(); + // If STATIC_LINK_MSVCRT feature isn't specified by user, we add DYNAMIC_LINK_MSVCRT_* feature // according to compilation mode. // If STATIC_LINK_MSVCRT feature is specified, we add STATIC_LINK_MSVCRT_* feature @@ -713,6 +715,38 @@ public final class CcCommon { ? CppRuleClasses.DYNAMIC_LINK_MSVCRT_DEBUG : CppRuleClasses.DYNAMIC_LINK_MSVCRT_NO_DEBUG); } + + CppConfiguration cppConfiguration = toolchain.getCppConfiguration(); + if (cppConfiguration.getFdoInstrument() != null + && !ruleContext.getDisabledFeatures().contains(CppRuleClasses.FDO_INSTRUMENT)) { + allRequestedFeaturesBuilder.add(CppRuleClasses.FDO_INSTRUMENT); + } + + FdoMode fdoMode = toolchain.getFdoMode(); + boolean isFdo = fdoMode != FdoMode.OFF && toolchain.getCompilationMode() == CompilationMode.OPT; + if (isFdo + && fdoMode != FdoMode.AUTO_FDO + && !ruleContext.getDisabledFeatures().contains(CppRuleClasses.FDO_OPTIMIZE)) { + allRequestedFeaturesBuilder.add(CppRuleClasses.FDO_OPTIMIZE); + } + if (isFdo && fdoMode == FdoMode.AUTO_FDO) { + allRequestedFeaturesBuilder.add(CppRuleClasses.AUTOFDO); + // For LLVM, support implicit enabling of ThinLTO for AFDO unless it has been + // explicitly disabled. + if (toolchain.isLLVMCompiler() + && !ruleContext.getDisabledFeatures().contains(CppRuleClasses.THIN_LTO)) { + allRequestedFeaturesBuilder.add(CppRuleClasses.ENABLE_AFDO_THINLTO); + } + } + if (cppConfiguration.isLipoOptimizationOrInstrumentation()) { + // Map LIPO to ThinLTO for LLVM builds. + if (toolchain.isLLVMCompiler() && fdoMode != FdoMode.OFF) { + allRequestedFeaturesBuilder.add(CppRuleClasses.THIN_LTO); + } else { + allRequestedFeaturesBuilder.add(CppRuleClasses.LIPO); + } + } + ImmutableList.Builder<String> allFeatures = new ImmutableList.Builder<String>() .addAll( 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 885f031fac..22ee5acfa6 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 @@ -53,6 +53,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.Builder; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoMode; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; @@ -371,12 +372,20 @@ public class CcToolchain implements RuleConfiguredTargetFactory { return null; } + FdoMode fdoMode; + if (fdoZip == null) { + fdoMode = FdoMode.OFF; + } else if (CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip.getBaseName())) { + fdoMode = FdoMode.AUTO_FDO; + } else if (cppConfiguration.isLLVMOptimizedFdo(toolchainInfo.isLLVMCompiler())) { + fdoMode = FdoMode.LLVM_FDO; + } else { + fdoMode = FdoMode.VANILLA; + } + SkyKey fdoKey = FdoSupportValue.key( - cppConfiguration.getLipoMode(), - fdoZip, - cppConfiguration.getFdoInstrument(), - cppConfiguration.isLLVMOptimizedFdo(toolchainInfo.isLLVMCompiler())); + cppConfiguration.getLipoMode(), fdoZip, cppConfiguration.getFdoInstrument(), fdoMode); SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv(); FdoSupportValue fdoSupport; @@ -562,7 +571,8 @@ public class CcToolchain implements RuleConfiguredTargetFactory { : null, getEnvironment(ruleContext), builtInIncludeDirectories, - sysroot); + sysroot, + fdoMode); TemplateVariableInfo templateVariableInfo = createMakeVariableProvider(cppConfiguration, sysroot); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index 2f4ca798f3..0f8358ec46 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoMode; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.util.Pair; @@ -79,7 +80,8 @@ public final class CcToolchainProvider extends ToolchainInfo { null, ImmutableMap.<String, String>of(), ImmutableList.<PathFragment>of(), - null); + null, + FdoMode.OFF); @Nullable private final CppConfiguration cppConfiguration; private final CppToolchainInfo toolchainInfo; @@ -109,6 +111,7 @@ public final class CcToolchainProvider extends ToolchainInfo { private final ImmutableMap<String, String> environment; private final ImmutableList<PathFragment> builtInIncludeDirectories; @Nullable private final PathFragment sysroot; + private final FdoMode fdoMode; public CcToolchainProvider( ImmutableMap<String, Object> skylarkToolchain, @@ -139,7 +142,8 @@ public final class CcToolchainProvider extends ToolchainInfo { Artifact linkDynamicLibraryTool, ImmutableMap<String, String> environment, ImmutableList<PathFragment> builtInIncludeDirectories, - @Nullable PathFragment sysroot) { + @Nullable PathFragment sysroot, + FdoMode fdoMode) { super(skylarkToolchain, Location.BUILTIN); this.cppConfiguration = cppConfiguration; this.toolchainInfo = toolchainInfo; @@ -169,6 +173,7 @@ public final class CcToolchainProvider extends ToolchainInfo { this.environment = environment; this.builtInIncludeDirectories = builtInIncludeDirectories; this.sysroot = sysroot; + this.fdoMode = fdoMode; } /** Returns c++ Make variables. */ @@ -782,6 +787,10 @@ public final class CcToolchainProvider extends ToolchainInfo { return toolchainInfo.isLLVMCompiler(); } + public FdoMode getFdoMode() { + return fdoMode; + } + // Not all of CcToolchainProvider is exposed to Skylark, which makes implementing deep equality // impossible: if Java-only parts are considered, the behavior is surprising in Skylark, if they // are not, the behavior is surprising in Java. Thus, object identity it is. 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 2049d04166..8302cc0380 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 @@ -1343,36 +1343,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { public ImmutableSet<String> configurationEnabledFeatures( RuleContext ruleContext, ImmutableSet<String> disabledFeatures) { ImmutableSet.Builder<String> requestedFeatures = ImmutableSet.builder(); - if (cppOptions.getFdoInstrument() != null) { - requestedFeatures.add(CppRuleClasses.FDO_INSTRUMENT); - } - - String fdoZip = null; - if (fdoProfileAbsolutePath != null) { - fdoZip = fdoProfileAbsolutePath.getPathString(); - } else if (fdoProfileLabel != null) { - fdoZip = fdoProfileLabel.getName(); - } - boolean isFdo = fdoZip != null && compilationMode == CompilationMode.OPT; - if (isFdo && !CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) { - requestedFeatures.add(CppRuleClasses.FDO_OPTIMIZE); - } - if (isFdo && CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) { - requestedFeatures.add(CppRuleClasses.AUTOFDO); - // For LLVM, support implicit enabling of ThinLTO for AFDO unless it has been - // explicitly disabled. - if (isLLVMCompiler() && !disabledFeatures.contains(CppRuleClasses.THIN_LTO)) { - requestedFeatures.add(CppRuleClasses.ENABLE_AFDO_THINLTO); - } - } - if (isLipoOptimizationOrInstrumentation()) { - // Map LIPO to ThinLTO for LLVM builds. - if (isLLVMCompiler() && cppOptions.getFdoOptimize() != null) { - requestedFeatures.add(CppRuleClasses.THIN_LTO); - } else { - requestedFeatures.add(CppRuleClasses.LIPO); - } - } if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { requestedFeatures.add(CppRuleClasses.COVERAGE); if (useLLVMCoverageMap) { 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 41b1605ec0..bbaa0bf8c5 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 @@ -283,20 +283,10 @@ public class FdoSupport { PathFragment fdoInstrument, Path fdoProfile, LipoMode lipoMode, - boolean llvmFdo, Path execRoot, - String productName) + String productName, + FdoMode fdoMode) throws IOException, FdoException, InterruptedException { - FdoMode fdoMode; - if (fdoProfile != null && isAutoFdo(fdoProfile.getBaseName())) { - fdoMode = FdoMode.AUTO_FDO; - } else if (fdoProfile != null && llvmFdo) { - fdoMode = FdoMode.LLVM_FDO; - } else if (fdoProfile != null) { - fdoMode = FdoMode.VANILLA; - } else { - fdoMode = FdoMode.OFF; - } if (fdoProfile == null) { lipoMode = LipoMode.OFF; 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 index 63c2d3f336..f864104b31 100644 --- 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 @@ -65,9 +65,9 @@ public class FdoSupportFunction implements SkyFunction { key.getFdoInstrument(), key.getFdoZip(), key.getLipoMode(), - key.getLLVMFdo(), execRoot, - directories.getProductName()); + directories.getProductName(), + key.getFdoMode()); if (env.valuesMissing()) { return null; } 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 index 0f94dcd486..6cd722f8e4 100644 --- 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 @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoMode; 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; @@ -44,13 +45,13 @@ public class FdoSupportValue implements SkyValue { private final LipoMode lipoMode; private final Path fdoZip; private final PathFragment fdoInstrument; - private final boolean llvmFdo; + private final FdoMode fdoMode; - private Key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument, boolean llvmFdo) { + private Key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument, FdoMode fdoMode) { this.lipoMode = lipoMode; this.fdoZip = fdoZip; this.fdoInstrument = fdoInstrument; - this.llvmFdo = llvmFdo; + this.fdoMode = fdoMode; } public LipoMode getLipoMode() { @@ -65,8 +66,8 @@ public class FdoSupportValue implements SkyValue { return fdoInstrument; } - public boolean getLLVMFdo() { - return llvmFdo; + public FdoMode getFdoMode() { + return fdoMode; } @Override @@ -82,7 +83,7 @@ public class FdoSupportValue implements SkyValue { Key that = (Key) o; return Objects.equals(this.lipoMode, that.lipoMode) && Objects.equals(this.fdoZip, that.fdoZip) - && Objects.equals(this.llvmFdo, that.llvmFdo) + && Objects.equals(this.fdoMode, that.fdoMode) && Objects.equals(this.fdoInstrument, that.fdoInstrument); } @@ -103,7 +104,7 @@ public class FdoSupportValue implements SkyValue { } public static SkyKey key( - LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument, boolean llvmFdo) { - return LegacySkyKey.create(SKYFUNCTION, new Key(lipoMode, fdoZip, fdoInstrument, llvmFdo)); + LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument, FdoMode fdoMode) { + return LegacySkyKey.create(SKYFUNCTION, new Key(lipoMode, fdoZip, fdoInstrument, fdoMode)); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java index 85b0c3d610..a6b67737c7 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; +import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoMode; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import org.junit.Test; @@ -64,7 +65,8 @@ public class CcToolchainProviderTest { null, ImmutableMap.<String, String>of(), ImmutableList.<PathFragment>of(), - null); + null, + FdoMode.OFF); CcToolchainProvider b = new CcToolchainProvider( @@ -96,7 +98,8 @@ public class CcToolchainProviderTest { null, ImmutableMap.<String, String>of(), ImmutableList.<PathFragment>of(), - null); + null, + FdoMode.OFF); new EqualsTester() .addEqualityGroup(a) |