Don't propagate FDO to exec actions.
Benefits:
- More efficient caching for exec actions (which shouldn't integrate FDO data)
- simpler, more straightforward code
- resolves outdated logic from before toolchain transitions (https://github.com/bazelbuild/proposals/blob/main/designs/2020-02-07-toolchain-transition-migration.md)
https://github.com/bazelbuild/bazel/issues/6516
PiperOrigin-RevId: 478011721
Change-Id: I8df049eeb78003fa9e979a91313e45cb9de95d01
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
index 2db6381..e0cc92d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
@@ -94,15 +94,13 @@
LabelLateBoundDefault.fromTargetConfiguration(
CppConfiguration.class,
null,
- (rule, attributes, cppConfig) ->
- cppConfig.getFdoOptimizeLabelUnsafeSinceItCanReturnValueFromWrongConfiguration());
+ (rule, attributes, cppConfig) -> cppConfig.getFdoOptimizeLabel());
private static final LabelLateBoundDefault<?> FDO_PROFILE_VALUE =
LabelLateBoundDefault.fromTargetConfiguration(
CppConfiguration.class,
null,
- (rule, attributes, cppConfig) ->
- cppConfig.getFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration());
+ (rule, attributes, cppConfig) -> cppConfig.getFdoProfileLabel());
private static final LabelLateBoundDefault<?> CSFDO_PROFILE_VALUE =
LabelLateBoundDefault.fromTargetConfiguration(
@@ -114,8 +112,7 @@
LabelLateBoundDefault.fromTargetConfiguration(
CppConfiguration.class,
null,
- (rule, attributes, cppConfig) ->
- cppConfig.getXFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration());
+ (rule, attributes, cppConfig) -> cppConfig.getXFdoProfileLabel());
private static final LabelLateBoundDefault<?> FDO_PREFETCH_HINTS =
LabelLateBoundDefault.fromTargetConfiguration(
@@ -134,20 +131,9 @@
* enabled through --fdo_optimize or --fdo_profile
*/
private static boolean shouldIncludeZipperInToolchain(CppConfiguration cppConfiguration) {
- if (cppConfiguration.isToolConfigurationDoNotUseWillBeRemovedFor129045294()) {
- // This is actually a bug, because with platforms, and before toolchain-transitions are
- // implemented, all toolchains are analyzed in the host configuration. We're betting on
- // nobody in the Bazel world actually using zipped fdo profiles and platforms at the same
- // time, and if people do, they'll have to wait for toolchain-transitions. This needs to be
- // fixed.
- // TODO(b/129045294): Fix this once toolchain-transitions are implemented.
- return false;
- }
- return cppConfiguration.getFdoOptimizeLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null
- || cppConfiguration.getFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null
- || cppConfiguration.getFdoPathUnsafeSinceItCanReturnValueFromWrongConfiguration() != null;
+ return cppConfiguration.getFdoOptimizeLabel() != null
+ || cppConfiguration.getFdoProfileLabel() != null
+ || cppConfiguration.getFdoPath() != null;
}
@Override
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 1b7b363..938ea7f 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
@@ -620,28 +620,14 @@
@Nullable
String getFdoInstrument() {
- if (isToolConfigurationDoNotUseWillBeRemovedFor129045294()) {
- // We don't want FDO in the host configuration
- return null;
- }
return cppOptions.fdoInstrumentForBuild;
}
- /**
- * @deprecated Unsafe because it returns a value from target configuration even in the host
- * configuration.
- */
- @Deprecated
- PathFragment getFdoPathUnsafeSinceItCanReturnValueFromWrongConfiguration() {
+ PathFragment getFdoPath() {
return fdoPath;
}
- /**
- * @deprecated Unsafe because it returns a value from target configuration even in the host
- * configuration.
- */
- @Deprecated
- Label getFdoOptimizeLabelUnsafeSinceItCanReturnValueFromWrongConfiguration() {
+ Label getFdoOptimizeLabel() {
return fdoOptimizeLabel;
}
@@ -663,28 +649,10 @@
@Nullable
Label getFdoPrefetchHintsLabel() {
- if (isToolConfigurationDoNotUseWillBeRemovedFor129045294()) {
- // We don't want FDO in the host configuration
- return null;
- }
- return getFdoPrefetchHintsLabelUnsafeSinceItCanReturnValueFromWrongConfiguration();
- }
-
- /**
- * @deprecated Unsafe because it returns a value from target configuration even in the host
- * configuration.
- */
- @Deprecated
- Label getFdoPrefetchHintsLabelUnsafeSinceItCanReturnValueFromWrongConfiguration() {
return cppOptions.getFdoPrefetchHintsLabel();
}
- /**
- * @deprecated Unsafe because it returns a value from target configuration even in the host
- * configuration.
- */
- @Deprecated
- Label getFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration() {
+ Label getFdoProfileLabel() {
return cppOptions.fdoProfileLabel;
}
@@ -692,30 +660,16 @@
return cppOptions.csFdoProfileLabel;
}
- public Label getPropellerOptimizeLabel() {
- return cppOptions.propellerOptimizeLabel;
- }
-
- /**
- * @deprecated Unsafe because it returns a value from target configuration even in the host
- * configuration.
- */
@Nullable
- @Deprecated
- Label getPropellerOptimizeLabelUnsafeSinceItCanReturnValueFromWrongConfiguration() {
+ Label getPropellerOptimizeLabel() {
if (cppOptions.fdoInstrumentForBuild != null || cppOptions.csFdoInstrumentForBuild != null) {
return null;
}
return cppOptions.getPropellerOptimizeLabel();
}
- /**
- * @deprecated Unsafe because it returns a value from target configuration even in the host
- * configuration.
- */
@Nullable
- @Deprecated
- Label getXFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration() {
+ Label getXFdoProfileLabel() {
if (cppOptions.fdoOptimizeForBuild != null
|| cppOptions.fdoInstrumentForBuild != null
|| cppOptions.fdoProfileLabel != null
@@ -770,7 +724,7 @@
*/
@Nullable
public Label getTargetLibcTopLabel() {
- if (!isToolConfigurationDoNotUseWillBeRemovedFor129045294()) {
+ if (!isToolConfigurationDoNotUseWillBeRemovedFor129045294) {
// This isn't for a platform-enabled C++ toolchain (legacy C++ toolchains evaluate in the
// target configuration while platform-enabled toolchains evaluate in the host/exec
// configuration). targetLibcTopLabel is only intended for platform-enabled toolchains and can
@@ -807,13 +761,6 @@
return collectCodeCoverage;
}
- /** @deprecated this is only a temporary workaround, will be removed by b/129045294. */
- // TODO(b/129045294): Remove at first opportunity
- @Deprecated
- boolean isToolConfigurationDoNotUseWillBeRemovedFor129045294() {
- return isToolConfigurationDoNotUseWillBeRemovedFor129045294;
- }
-
public boolean enableCcToolchainResolution() {
return cppOptions.enableCcToolchainResolution;
}
@@ -842,6 +789,7 @@
return cppOptions.validateTopLevelHeaderInclusions;
}
+ @Override
public boolean appleGenerateDsym() {
return appleGenerateDsym;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index 2be96b1..e5b3488 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -1209,13 +1209,8 @@
host.useStartEndLib = useStartEndLib;
host.stripBinaries = StripMode.ALWAYS;
- host.fdoOptimizeForBuild = fdoOptimizeForBuild;
- host.fdoProfileLabel = fdoProfileLabel;
- host.csFdoProfileLabel = csFdoProfileLabel;
- host.xfdoProfileLabel = xfdoProfileLabel;
host.inmemoryDotdFiles = inmemoryDotdFiles;
- host.enableFdoProfileAbsolutePath = enableFdoProfileAbsolutePath;
host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet;
host.disableLegacyCcProvider = disableLegacyCcProvider;
host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoContext.java
index f2ad038..62735cf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoContext.java
@@ -148,10 +148,6 @@
}
boolean hasArtifacts(CppConfiguration cppConfiguration) {
- if (cppConfiguration.isToolConfigurationDoNotUseWillBeRemovedFor129045294()) {
- // We don't want FDO for host configuration
- return false;
- }
return getBranchFdoProfile() != null
|| getPrefetchHintsArtifact() != null
|| getPropellerOptimizeInputFile() != null;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
index 19dbc04..df61601 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
@@ -53,9 +53,7 @@
Artifact protoProfileArtifact = null;
Pair<FdoInputFile, Artifact> fdoInputs = null;
if (configuration.getCompilationMode() == CompilationMode.OPT) {
- if (cppConfiguration
- .getFdoPrefetchHintsLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null) {
+ if (cppConfiguration.getFdoPrefetchHintsLabel() != null) {
FdoPrefetchHintsProvider provider = attributes.getFdoPrefetch();
prefetchHints = provider.getInputFile();
}
@@ -77,16 +75,13 @@
if (ccArtifact != null || ldArtifact != null) {
propellerOptimizeInputFile = new PropellerOptimizeInputFile(ccArtifact, ldArtifact);
}
- } else if (cppConfiguration
- .getPropellerOptimizeLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null) {
+ } else if (cppConfiguration.getPropellerOptimizeLabel() != null) {
PropellerOptimizeProvider provider = attributes.getPropellerOptimize();
propellerOptimizeInputFile = provider.getInputFile();
}
- if (cppConfiguration.getFdoPathUnsafeSinceItCanReturnValueFromWrongConfiguration() != null) {
- PathFragment fdoZip =
- cppConfiguration.getFdoPathUnsafeSinceItCanReturnValueFromWrongConfiguration();
+ if (cppConfiguration.getFdoPath() != null) {
+ PathFragment fdoZip = cppConfiguration.getFdoPath();
SkyKey fdoKey = CcSkyframeFdoSupportValue.key(fdoZip);
SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
CcSkyframeFdoSupportValue ccSkyframeFdoSupportValue =
@@ -99,22 +94,16 @@
Preconditions.checkState(fdoInputFile == null);
fdoInputFile =
FdoInputFile.fromAbsolutePath(ccSkyframeFdoSupportValue.getFdoZipPath().asFragment());
- } else if (cppConfiguration
- .getFdoOptimizeLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null) {
+ } else if (cppConfiguration.getFdoOptimizeLabel() != null) {
FdoProfileProvider fdoProfileProvider = attributes.getFdoOptimizeProvider();
if (fdoProfileProvider != null) {
fdoInputs = getFdoInputs(ruleContext, fdoProfileProvider);
} else {
fdoInputFile = fdoInputFileFromArtifacts(ruleContext, attributes);
}
- } else if (cppConfiguration
- .getFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null) {
+ } else if (cppConfiguration.getFdoProfileLabel() != null) {
fdoInputs = getFdoInputs(ruleContext, attributes.getFdoProfileProvider());
- } else if (cppConfiguration
- .getXFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null) {
+ } else if (cppConfiguration.getXFdoProfileLabel() != null) {
fdoInputs = getFdoInputs(ruleContext, attributes.getXFdoProfileProvider());
}
@@ -173,8 +162,7 @@
}
if ((branchFdoMode != BranchFdoMode.XBINARY_FDO)
&& (branchFdoMode != BranchFdoMode.AUTO_FDO)
- && cppConfiguration.getXFdoProfileLabelUnsafeSinceItCanReturnValueFromWrongConfiguration()
- != null) {
+ && cppConfiguration.getXFdoProfileLabel() != null) {
ruleContext.throwWithRuleError("--xbinary_fdo only accepts *.xfdo and *.afdo");
}
@@ -185,8 +173,7 @@
Artifact profileArtifact = null;
if (branchFdoMode == BranchFdoMode.LLVM_FDO) {
profileArtifact =
- convertLLVMRawProfileToIndexed(
- attributes, fdoInputFile, toolPaths, ruleContext, cppConfiguration, "fdo");
+ convertLLVMRawProfileToIndexed(attributes, fdoInputFile, toolPaths, ruleContext, "fdo");
if (ruleContext.hasErrors()) {
return null;
}
@@ -202,14 +189,13 @@
"Symlinking FDO profile " + fdoInputFile.getBasename());
} else if (branchFdoMode == BranchFdoMode.LLVM_CS_FDO) {
Artifact nonCSProfileArtifact =
- convertLLVMRawProfileToIndexed(
- attributes, fdoInputFile, toolPaths, ruleContext, cppConfiguration, "fdo");
+ convertLLVMRawProfileToIndexed(attributes, fdoInputFile, toolPaths, ruleContext, "fdo");
if (ruleContext.hasErrors()) {
return null;
}
Artifact csProfileArtifact =
convertLLVMRawProfileToIndexed(
- attributes, csFdoInputFile, toolPaths, ruleContext, cppConfiguration, "csfdo");
+ attributes, csFdoInputFile, toolPaths, ruleContext, "csfdo");
if (ruleContext.hasErrors()) {
return null;
}
@@ -342,12 +328,7 @@
FdoInputFile fdoProfile,
ImmutableMap<String, PathFragment> toolPaths,
RuleContext ruleContext,
- CppConfiguration cppConfiguration,
String fdoUniqueArtifactName) {
- if (cppConfiguration.isToolConfigurationDoNotUseWillBeRemovedFor129045294()) {
- return null;
- }
-
Artifact profileArtifact =
ruleContext.getUniqueDirectoryArtifact(
fdoUniqueArtifactName,
