diff options
3 files changed, 31 insertions, 17 deletions
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 563713933c..29831acc1c 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 @@ -67,6 +67,16 @@ public final class CcToolchainRule implements RuleDefinition { null, (rule, attributes, cppConfig) -> cppConfig.getFdoProfileLabel()); + /** + * Returns true if zipper should be loaded. We load the zipper executable if FDO optimization is + * enabled through --fdo_optimize or --fdo_profile + */ + private static boolean shouldIncludeZipperInToolchain(CppConfiguration cppConfiguration) { + return cppConfiguration.getFdoOptimizeLabel() != null + || cppConfiguration.getFdoProfileLabel() != null + || cppConfiguration.getFdoPath() != null; + } + @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { final Label zipper = env.getToolsLabel("//tools/zip:zipper"); @@ -137,9 +147,8 @@ public final class CcToolchainRule implements RuleDefinition { LabelLateBoundDefault.fromTargetConfiguration( CppConfiguration.class, null, - // TODO(b/69547565): Remove call to shouldIncludeZipperInToolchain (rule, attributes, cppConfig) -> - cppConfig.shouldIncludeZipperInToolchain() ? zipper : null))) + shouldIncludeZipperInToolchain(cppConfig) ? zipper : null))) .add(attr(":libc_top", LABEL).value(LIBC_TOP)) .add(attr(":fdo_optimize", LABEL).singleArtifact().value(FDO_OPTIMIZE_LABEL)) .add( 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 6f262410c2..4761c2b6d4 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 @@ -890,21 +890,6 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { return cppToolchainInfo.isLLVMCompiler(); } - /** - * Returns true if LLVM FDO Optimization should be applied for this configuration. - * - * <p>Deprecated: Use {@link CcToolchain#isLLVMOptimizedFdo(boolean, PathFragment)} - */ - // TODO(b/64384912): Remove in favor of overload with isLLVMCompiler. - @Deprecated - public boolean shouldIncludeZipperInToolchain() { - return (cppOptions.getFdoOptimize() != null - && (CppFileTypes.LLVM_PROFILE.matches(cppOptions.getFdoOptimize()) - || CppFileTypes.LLVM_PROFILE_RAW.matches(cppOptions.getFdoOptimize()) - || (isLLVMCompiler() && cppOptions.getFdoOptimize().endsWith(".zip")))) - || (cppOptions.getFdoProfileLabel() != null); - } - /** Returns true if LIPO optimization is implied by the flags of this build. */ public boolean lipoOptimizationIsActivated() { return cppOptions.isLipoOptimization(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java index afa366beef..58ea8940cc 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java @@ -714,6 +714,26 @@ public class CcToolchainTest extends BuildViewTestCase { } @Test + public void testZipperInclusionDependsOnFdoOptimization() throws Exception { + reporter.removeHandler(failFastHandler); + writeDummyCcToolchain(); + scratch.file("fdo/my_profile.afdo", ""); + scratch.file( + "fdo/BUILD", + "exports_files(['my_profile.afdo'])", + "fdo_profile(name = 'fdo', profile = ':my_profile.profdata')"); + + useConfiguration(); + assertThat(getPrerequisites(getConfiguredTarget("//a:b"), ":zipper")).isEmpty(); + + useConfiguration("-c", "opt", "--fdo_optimize=//fdo:my_profile.afdo"); + assertThat(getPrerequisites(getConfiguredTarget("//a:b"), ":zipper")).isNotEmpty(); + + useConfiguration("-c", "opt", "--fdo_profile=//fdo:fdo"); + assertThat(getPrerequisites(getConfiguredTarget("//a:b"), ":zipper")).isNotEmpty(); + } + + @Test public void testInlineCtoolchain_withoutToolchainResolution() throws Exception { scratch.file( "a/BUILD", |