Convert LLVM raw profiles to indexed format if necessary.

Here is what is done today:

bazel build -c opt --fdo_optimize=<path_to_profdata_file> //target

The .profdata file is the LLVM profiles in indexed format and bazel creates a
symlink to it from bazel-fdo/_fdo/...  and compiles the target.

However, the instrumented binary generates a .profraw file and hence the
conversion to .profdata must be done manually using the llvm-profdata binary
which is shipped along with the crosstool as:
$ llvm-profdata -merge -o <path_to_profdata_file> <path_profraw_file>

We are trying to avoid this intermediate step in this change by baking this
into bazel.

This implementation does the following:

* In CppConfiguration.java, adds  new tool llvm-profdata.
* In CcToolchain.java, in function create, if LLVM instrumented FDO is desired,
  the profile format is checked and the profile conversion takes place.
* FdoSupport.java checks for LLVM instrumented FDO is bypassed.

RELNOTES[NEW]: Raw LLVM profiles are now supported.

PiperOrigin-RevId: 154569896

6 files changed, 163 insertions, 41 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java
index 20b2266340..6cace67a03 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java
@@ -73,6 +73,25 @@ public class SymlinkAction extends AbstractAction {
     this.progressMessage = progressMessage;
   }
 
+  /**
+   * Creates a new SymlinkAction instance, where an input artifact is not present. This is useful
+   * when dealing with special cases where input paths that are outside the exec root directory
+   * tree. Currently, the only instance where this happens is for FDO builds where the profile file
+   * is outside the exec root structure.
+   *
+   * @param owner the action owner.
+   * @param inputPath the Path that will be the src of the symbolic link.
+   * @param output the Artifact that will be created by executing this Action.
+   * @param progressMessage the progress message.
+   */
+  public SymlinkAction(
+      ActionOwner owner, PathFragment inputPath, Artifact output, String progressMessage) {
+    super(owner, Artifact.NO_ARTIFACTS, ImmutableList.of(output));
+    this.inputPath = Preconditions.checkNotNull(inputPath);
+    this.output = Preconditions.checkNotNull(output);
+    this.progressMessage = progressMessage;
+  }
+
   public PathFragment getInputPath() {
     return inputPath;
   }
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 cc01e6c7f4..14da7d1b21 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,6 +33,8 @@ 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.actions.SpawnAction;
+import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 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;
@@ -44,6 +46,7 @@ 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.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -65,6 +68,72 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
   private static final PathFragment BUILTIN_INCLUDE_FILE_SUFFIX =
       PathFragment.create("include/stdc-predef.h");
 
+  private static String getLLVMProfileFileName(Path fdoProfile) {
+    if (CppFileTypes.LLVM_PROFILE.matches(fdoProfile)) {
+      return fdoProfile.getBaseName();
+    } else {
+      return FileSystemUtils.removeExtension(fdoProfile.getBaseName())
+          + CppFileTypes.LLVM_PROFILE.getExtensions().get(0);
+    }
+  }
+
+  /*
+   * This function checks the format of the input profile data and converts it to
+   * the indexed format (.profdata) if necessary.
+   */
+  private Artifact convertLLVMRawProfileToIndexed(
+      Path fdoProfile, CppConfiguration cppConfiguration, RuleContext ruleContext)
+      throws InterruptedException {
+
+    Artifact profileArtifact =
+        ruleContext.getUniqueDirectoryArtifact(
+            "fdo", getLLVMProfileFileName(fdoProfile), ruleContext.getBinOrGenfilesDirectory());
+
+    // If the profile file is already in the desired format, symlink to it and return.
+    if (CppFileTypes.LLVM_PROFILE.matches(fdoProfile)) {
+      ruleContext.registerAction(
+          new SymlinkAction(
+              ruleContext.getActionOwner(),
+              PathFragment.create(fdoProfile.getPathString()),
+              profileArtifact,
+              "Symlinking LLVM Profile " + fdoProfile.getPathString()));
+      return profileArtifact;
+    }
+
+    Artifact rawProfileArtifact =
+        ruleContext.getUniqueDirectoryArtifact(
+            "fdo", fdoProfile.getBaseName(), ruleContext.getBinOrGenfilesDirectory());
+
+    ruleContext.registerAction(
+        new SymlinkAction(
+            ruleContext.getActionOwner(),
+            PathFragment.create(fdoProfile.getPathString()),
+            rawProfileArtifact,
+            "Symlinking LLVM Profile " + fdoProfile.getPathString()));
+
+    if (cppConfiguration.getLLVMProfDataExecutable() == null) {
+      ruleContext.ruleError(
+          "llvm-profdata not available with this crosstool, needed for profile conversion");
+      return null;
+    }
+
+    // Convert LLVM raw profile to indexed format.
+    ruleContext.registerAction(
+        new SpawnAction.Builder()
+            .addInput(rawProfileArtifact)
+            .addTransitiveInputs(getFiles(ruleContext, "all_files"))
+            .addOutput(profileArtifact)
+            .useDefaultShellEnvironment()
+            .setExecutable(cppConfiguration.getLLVMProfDataExecutable())
+            .addArguments("merge", "-o", profileArtifact.getExecPathString())
+            .addArgument(rawProfileArtifact.getExecPathString())
+            .setProgressMessage("LLVMProfDataAction: Generating " + profileArtifact.prettyPrint())
+            .setMnemonic("LLVMProfDataAction")
+            .build(ruleContext));
+
+    return profileArtifact;
+  }
+
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws RuleErrorException, InterruptedException {
@@ -100,6 +169,15 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
       return null;
     }
 
+    // This tries to convert LLVM profiles to the indexed format if necessary.
+    Artifact profileArtifact = null;
+    if (cppConfiguration.isLLVMOptimizedFdo()) {
+      profileArtifact = convertLLVMRawProfileToIndexed(fdoZip, cppConfiguration, ruleContext);
+      if (ruleContext.hasErrors()) {
+        return null;
+      }
+    }
+
     final Label label = ruleContext.getLabel();
     final NestedSet<Artifact> crosstool = ruleContext.getPrerequisite("all_files", Mode.HOST)
         .getProvider(FileProvider.class).getFilesToBuild();
@@ -246,7 +324,8 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
             .addNativeDeclaredProvider(ccProvider)
             .addProvider(makeVariableProvider)
             .addNativeDeclaredProvider(makeVariableProvider)
-            .addProvider(fdoSupport.getFdoSupport().createFdoSupportProvider(ruleContext))
+            .addProvider(
+                fdoSupport.getFdoSupport().createFdoSupportProvider(ruleContext, profileArtifact))
             .setFilesToBuild(new NestedSetBuilder<Artifact>(Order.STABLE_ORDER).build())
             .addProvider(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 c924b09daf..881dd0a8ce 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
@@ -100,7 +100,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     OBJCOPY("objcopy"),
     OBJDUMP("objdump"),
     STRIP("strip"),
-    DWP("dwp");
+    DWP("dwp"),
+    LLVM_PROFDATA("llvm-profdata");
 
     private final String namePart;
 
@@ -516,21 +517,27 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             crosstoolTopPathFragment.getRelative(tool.getNamePart()));
       }
     } else {
-      Iterable<Tool> neededTools = Iterables.filter(EnumSet.allOf(Tool.class),
-          new Predicate<Tool>() {
-            @Override
-            public boolean apply(Tool tool) {
-              if (tool == Tool.DWP) {
-                // When fission is unsupported, don't check for the dwp tool.
-                return supportsFission();
-              } else if (tool == Tool.GCOVTOOL || tool == Tool.OBJCOPY) {
-                // gcov-tool and objcopy are optional, don't check whether they're present
-                return false;
-              } else {
-                return true;
-              }
-            }
-          });
+      Iterable<Tool> neededTools =
+          Iterables.filter(
+              EnumSet.allOf(Tool.class),
+              new Predicate<Tool>() {
+                @Override
+                public boolean apply(Tool tool) {
+                  if (tool == Tool.DWP) {
+                    // When fission is unsupported, don't check for the dwp tool.
+                    return supportsFission();
+                  } else if (tool == Tool.LLVM_PROFDATA) {
+                    // TODO(tmsriram): Fix this to check if this is a llvm crosstool
+                    // and return true.  This needs changes to crosstool_config.proto.
+                    return false;
+                  } else if (tool == Tool.GCOVTOOL || tool == Tool.OBJCOPY) {
+                    // gcov-tool and objcopy are optional, don't check whether they're present
+                    return false;
+                  } else {
+                    return true;
+                  }
+                }
+              });
       for (Tool tool : neededTools) {
         if (!toolPaths.containsKey(tool.getNamePart())) {
           throw new IllegalArgumentException("Tool path for '" + tool.getNamePart()
@@ -1709,6 +1716,14 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return cppOptions.isFdo();
   }
 
+  /** Returns true if LLVM FDO Optimization should be applied for this configuration. */
+  public boolean isLLVMOptimizedFdo() {
+    return cppOptions.isFdo()
+        && cppOptions.getFdoOptimize() != null
+        && (CppFileTypes.LLVM_PROFILE.matches(cppOptions.getFdoOptimize())
+            || CppFileTypes.LLVM_PROFILE_RAW.matches(cppOptions.getFdoOptimize()));
+  }
+
   /**
    * Returns true if LIPO optimization should be applied for this configuration.
    */
@@ -2032,6 +2047,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return getToolPathFragment(CppConfiguration.Tool.DWP);
   }
 
+  public PathFragment getLLVMProfDataExecutable() {
+    return getToolPathFragment(CppConfiguration.Tool.LLVM_PROFDATA);
+  }
+
   /**
    * Returns the GNU System Name
    */
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 597ebed521..cdda812667 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
@@ -156,6 +156,7 @@ public final class CppFileTypes {
   public static final FileType COVERAGE_DATA_IMPORTS = FileType.of(".gcda.imports");
   public static final FileType GCC_AUTO_PROFILE = FileType.of(".afdo");
   public static final FileType LLVM_PROFILE = FileType.of(".profdata");
+  public static final FileType LLVM_PROFILE_RAW = FileType.of(".profraw");
 
   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/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index 230b095fd8..a4e2401e94 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
@@ -398,7 +398,7 @@ public class CppOptions extends FragmentOptions {
             + "an auto profile. This flag also accepts files specified as labels, for "
             + "example //foo/bar:file.afdo. Such labels must refer to input files; you may "
             + "need to add an exports_files directive to the corresponding package to make "
-            + "the file visible to Blaze. It also accepts an indexed LLVM profile file."
+            + "the file visible to Blaze. It also accepts a raw or an indexed LLVM profile file."
   )
   /**
    * Never read FDO/LIPO options directly. This is because {@link #lipoConfigurationState}
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 4d27ba4365..eddb0452ed 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
@@ -158,7 +158,8 @@ public class FdoSupport {
    * Returns true if the given fdoFile represents an LLVM profile.
    */
   public static final boolean isLLVMFdo(String fdoFile) {
-    return CppFileTypes.LLVM_PROFILE.matches(fdoFile);
+    return (CppFileTypes.LLVM_PROFILE.matches(fdoFile)
+        || CppFileTypes.LLVM_PROFILE_RAW.matches(fdoFile));
   }
 
   /**
@@ -242,8 +243,13 @@ public class FdoSupport {
             fdoProfile.getBaseName()));
     this.lipoMode = lipoMode;
     this.fdoMode = fdoMode;
-    this.gcdaFiles = fdoZipContents.gcdaFiles;
-    this.imports = fdoZipContents.imports;
+    if (fdoZipContents != null) {
+      this.gcdaFiles = fdoZipContents.gcdaFiles;
+      this.imports = fdoZipContents.imports;
+    } else {
+      this.gcdaFiles = null;
+      this.imports = null;
+    }
   }
 
   public Root getFdoRoot() {
@@ -301,6 +307,11 @@ public class FdoSupport {
       return null;
     }
 
+    if (fdoMode == FdoMode.LLVM_FDO) {
+      return new FdoSupport(
+          fdoMode, LipoMode.OFF, fdoRoot, fdoRootExecPath, fdoInstrument, fdoProfile, null);
+    }
+    
     FdoZipContents fdoZipContents = extractFdoZip(
         fdoMode, lipoMode, execRoot, fdoProfile, fdoRootExecPath,
         PrecomputedValue.PRODUCT_NAME.get(env));
@@ -349,9 +360,6 @@ public class FdoSupport {
         }
         FileSystemUtils.ensureSymbolicLink(
             execRoot.getRelative(getAutoProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile);
-      } else if (fdoMode == FdoMode.LLVM_FDO) {
-        FileSystemUtils.ensureSymbolicLink(
-            execRoot.getRelative(getLLVMProfilePath(fdoProfile, fdoRootExecPath)), fdoProfile);
       } else {
         Path zipFilePath = new ZipFileSystem(fdoProfile).getRootDirectory();
         String outputSymlinkName = productName + "-out";
@@ -556,8 +564,7 @@ public class FdoSupport {
         if (featureConfiguration.isEnabled(CppRuleClasses.FDO_OPTIMIZE)) {
           if (fdoMode == FdoMode.LLVM_FDO) {
             buildVariables.addStringVariable(
-                "fdo_profile_path",
-                getLLVMProfilePath(fdoProfile, fdoRootExecPath).getPathString());
+                "fdo_profile_path", fdoSupportProvider.getProfileArtifact().getExecPathString());
           } else {
             buildVariables.addStringVariable("fdo_profile_path", fdoRootExecPath.getPathString());
           }
@@ -684,15 +691,6 @@ public class FdoSupport {
     return PathFragment.create(fdoProfile.getBaseName());
   }
 
-
-  private static PathFragment getLLVMProfilePath(Path fdoProfile, PathFragment fdoRootExecPath) {
-    return fdoRootExecPath.getRelative(getLLVMProfileRootRelativePath(fdoProfile));
-  }
-
-  private static PathFragment getLLVMProfileRootRelativePath(Path fdoProfile) {
-    return PathFragment.create(fdoProfile.getBaseName());
-  }
-
   /**
    * Returns whether AutoFDO is enabled.
    */
@@ -741,17 +739,23 @@ public class FdoSupport {
   }
 
   public FdoSupportProvider createFdoSupportProvider(
-      RuleContext ruleContext) {
+      RuleContext ruleContext, Artifact profileArtifact) {
     if (fdoRoot == null) {
       return new FdoSupportProvider(this, null, null);
     }
 
+    if (fdoMode == FdoMode.LLVM_FDO) {
+      Preconditions.checkState(profileArtifact != null);
+      return new FdoSupportProvider(this, profileArtifact, null);
+    }
+
     Preconditions.checkState(fdoPath != null);
-    PathFragment profileRootRelativePath = fdoMode == FdoMode.LLVM_FDO
-        ? getLLVMProfileRootRelativePath(fdoProfile)
-        : getAutoProfileRootRelativePath(fdoProfile);
-    Artifact profileArtifact = ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-        fdoPath.getRelative(profileRootRelativePath), fdoRoot);
+    PathFragment profileRootRelativePath = getAutoProfileRootRelativePath(fdoProfile);
+
+    profileArtifact =
+        ruleContext
+            .getAnalysisEnvironment()
+            .getDerivedArtifact(fdoPath.getRelative(profileRootRelativePath), fdoRoot);
     ruleContext.registerAction(new FdoStubAction(ruleContext.getActionOwner(), profileArtifact));
     Preconditions.checkState(fdoPath != null);
     ImmutableMap.Builder<PathFragment, Artifact> gcdaArtifacts = ImmutableMap.builder();