Remove FdoMode.VANILLA.
And a little bit of associated refactoring.
RELNOTES: None.
PiperOrigin-RevId: 210097491
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 855998b..9962f29 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -146,19 +146,8 @@
}
// If --fdo_optimize was not specified, we don't have any additional inputs.
- switch (fdoSupportProvider.getFdoMode()) {
- case LLVM_FDO:
- case AUTO_FDO:
- case XBINARY_FDO:
- auxiliaryInputs.add(fdoSupportProvider.getProfileArtifact());
- break;
-
- case VANILLA:
- case OFF:
- break;
-
- default:
- throw new IllegalStateException();
+ if (fdoSupportProvider.getFdoMode() != FdoMode.OFF) {
+ auxiliaryInputs.add(fdoSupportProvider.getProfileArtifact());
}
return auxiliaryInputs.build();
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 07117a9..3d68b2d 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
@@ -57,7 +57,6 @@
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;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -232,7 +231,7 @@
Artifact rawProfileArtifact;
- if (fdoProfile.getBaseName().endsWith(".zip")) {
+ if (CppFileTypes.LLVM_PROFILE_ZIP.matches(fdoProfile)) {
// Get the zipper binary for unzipping the profile.
Artifact zipperBinaryArtifact = ruleContext.getPrerequisiteArtifact(":zipper", Mode.HOST);
if (zipperBinaryArtifact == null) {
@@ -371,29 +370,22 @@
}
}
- FileTypeSet validExtensions =
- FileTypeSet.of(
- CppFileTypes.GCC_AUTO_PROFILE,
- CppFileTypes.XBINARY_PROFILE,
- CppFileTypes.LLVM_PROFILE,
- CppFileTypes.LLVM_PROFILE_RAW,
- FileType.of(".zip"));
- if (fdoZip != null && !validExtensions.matches(fdoZip.getPathString())) {
- ruleContext.ruleError("invalid extension for FDO profile file.");
- return null;
- }
-
FdoMode fdoMode;
if (fdoZip == null) {
fdoMode = FdoMode.OFF;
- } else if (CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip.getBaseName())) {
+ } else if (CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) {
fdoMode = FdoMode.AUTO_FDO;
- } else if (isLLVMOptimizedFdo(toolchainInfo.isLLVMCompiler(), fdoZip)) {
- fdoMode = FdoMode.LLVM_FDO;
- } else if (CppFileTypes.XBINARY_PROFILE.matches(fdoZip.getBaseName())) {
+ } else if (CppFileTypes.XBINARY_PROFILE.matches(fdoZip)) {
fdoMode = FdoMode.XBINARY_FDO;
+ } else if (CppFileTypes.LLVM_PROFILE.matches(fdoZip)) {
+ fdoMode = FdoMode.LLVM_FDO;
+ } else if (CppFileTypes.LLVM_PROFILE_RAW.matches(fdoZip)) {
+ fdoMode = FdoMode.LLVM_FDO;
+ } else if (CppFileTypes.LLVM_PROFILE_ZIP.matches(fdoZip)) {
+ fdoMode = FdoMode.LLVM_FDO;
} else {
- fdoMode = FdoMode.VANILLA;
+ ruleContext.ruleError("invalid extension for FDO profile file.");
+ return null;
}
SkyKey fdoKey = FdoSupportValue.key(fdoZip);
@@ -631,14 +623,6 @@
return builder.build();
}
- /** Returns true if LLVM FDO Optimization should be applied for this configuration. */
- private boolean isLLVMOptimizedFdo(boolean isLLVMCompiler, PathFragment fdoProfilePath) {
- return fdoProfilePath != null
- && (CppFileTypes.LLVM_PROFILE.matches(fdoProfilePath)
- || CppFileTypes.LLVM_PROFILE_RAW.matches(fdoProfilePath)
- || (isLLVMCompiler && fdoProfilePath.toString().endsWith(".zip")));
- }
-
/** Finds an appropriate {@link CppToolchainInfo} for this target. */
private CppToolchainInfo getCppToolchainInfo(
RuleContext ruleContext, CppConfiguration cppConfiguration) throws RuleErrorException {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
index c32b89e..5f9e5cf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
@@ -184,6 +184,7 @@
public static final FileType XBINARY_PROFILE = FileType.of(".xfdo");
public static final FileType LLVM_PROFILE = FileType.of(".profdata");
public static final FileType LLVM_PROFILE_RAW = FileType.of(".profraw");
+ public static final FileType LLVM_PROFILE_ZIP = FileType.of(".zip");
public static final FileType CPP_MODULE_MAP = FileType.of(".cppmap");
public static final FileType CPP_MODULE = FileType.of(".pcm");
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoProfileRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoProfileRule.java
index 9f46a3d..06a800a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoProfileRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoProfileRule.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.syntax.Type;
-import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.FileTypeSet;
/** {@code fdo_profile} rule class. */
@@ -42,9 +41,9 @@
FileTypeSet.of(
CppFileTypes.LLVM_PROFILE_RAW,
CppFileTypes.LLVM_PROFILE,
+ CppFileTypes.LLVM_PROFILE_ZIP,
CppFileTypes.GCC_AUTO_PROFILE,
- CppFileTypes.XBINARY_PROFILE,
- FileType.of(".zip")))
+ CppFileTypes.XBINARY_PROFILE))
.singleArtifact())
/* <!-- #BLAZE_RULE(fdo_profile).ATTRIBUTE(absolute_path_profile) -->
Absolute path to the FDO profile. The FDO file can have one of the following extensions:
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 fcb0649..6c89bc2 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
@@ -38,9 +38,6 @@
/** FDO is turned off. */
OFF,
- /** Profiling-based FDO using an explicitly recorded profile. */
- VANILLA,
-
/** FDO based on automatically collected data. */
AUTO_FDO,
@@ -52,9 +49,7 @@
}
/**
- * Path of the profile file passed to {@code --fdo_optimize}, or
- * {@code null} if FDO optimization is disabled. The profile file
- * can be a coverage ZIP or an AutoFDO feedback file.
+ * Path of the profile file passed to {@code --fdo_optimize}
*/
// TODO(lberki): This should be a PathFragment.
// Except that CcProtoProfileProvider#getProfile() calls #exists() on it, which is ridiculously
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL b/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL
index c423ae2..74ba259 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL
@@ -83,6 +83,7 @@
objcopy_embed_flag: "binary"
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
# Anticipated future default.
unfiltered_cxx_flag: "-no-canonical-prefixes"
@@ -181,6 +182,7 @@
tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
needsPic: true
supports_fission: true
@@ -220,6 +222,7 @@
tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
needsPic: true
supports_fission: true
@@ -259,6 +262,7 @@
tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
needsPic: false
@@ -297,6 +301,7 @@
tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
needsPic: false
@@ -336,6 +341,7 @@
tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
needsPic: false
@@ -376,6 +382,7 @@
tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
tool_path { name: "objdump" path: "/usr/bin/objdump" }
tool_path { name: "strip" path: "/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "/usr/bin/llvm-profdata" }
needsPic: false
@@ -416,6 +423,7 @@
tool_path { name: "objcopy" path: "C:/mingw/bin/objcopy" }
tool_path { name: "objdump" path: "C:/mingw/bin/objdump" }
tool_path { name: "strip" path: "C:/mingw/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "C:/mingw/bin/llvm-profdata" }
builtin_sysroot: ""
cxx_flag: "-std=c++0x"
@@ -450,6 +458,7 @@
tool_path { name: "objcopy" path: "C:/msys64/mingw64/bin/objcopy" }
tool_path { name: "objdump" path: "C:/msys64/mingw64/bin/objdump" }
tool_path { name: "strip" path: "C:/msys64/mingw64/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "C:/msys64/mingw64/bin/llvm-profdata" }
builtin_sysroot: "/usr/grte/v1"
cxx_flag: "-std=c++0x"
@@ -483,6 +492,7 @@
tool_path { name: "objcopy" path: "C:/Program Files (x86)/LLVM/bin/objcopy" }
tool_path { name: "objdump" path: "C:/Program Files (x86)/LLVM/bin/objdump" }
tool_path { name: "strip" path: "C:/Program Files (x86)/LLVM/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "C:/Program Files (x86)/LLVM/bin/llvm-profdata" }
cxx_flag: "-std=c++0x"
cxx_builtin_include_directory: "/usr/lib/gcc/"
@@ -533,6 +543,7 @@
tool_path { name: "objcopy" path: "/bin/false" }
tool_path { name: "objdump" path: "/bin/false" }
tool_path { name: "strip" path: "/bin/false" }
+ tool_path { name: "llvm-profdata" path: "/bin/false" }
supports_interface_shared_objects: true
}
@@ -571,6 +582,7 @@
objcopy_embed_flag: "binary"
tool_path { name: "objdump" path: "C:/tools/msys64/usr/bin/objdump" }
tool_path { name: "strip" path: "C:/tools/msys64/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "C:/tools/msys64/usr/bin/llvm-profdata" }
linking_mode_flags { mode: DYNAMIC }
}
@@ -608,5 +620,6 @@
objcopy_embed_flag: "binary"
tool_path { name: "objdump" path: "C:/tools/msys64/usr/bin/objdump" }
tool_path { name: "strip" path: "C:/tools/msys64/usr/bin/strip" }
+ tool_path { name: "llvm-profdata" path: "C:/tools/msys64/usr/bin/llvm-profdata" }
# No linking_mode_flags here
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/Crosstool.java b/src/test/java/com/google/devtools/build/lib/packages/util/Crosstool.java
index ad799b9..cb2740b 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/Crosstool.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/Crosstool.java
@@ -29,7 +29,7 @@
*/
final class Crosstool {
private static final ImmutableList<String> CROSSTOOL_BINARIES =
- ImmutableList.of("ar", "as", "compile", "dwp", "link", "objcopy");
+ ImmutableList.of("ar", "as", "compile", "dwp", "link", "objcopy", "llvm-profdata");
private final MockToolsConfig config;
