[BOLT] Expose pseudo probe function checksum and GUID

[aaupov]

Add a BinaryFunction field for pseudo probe function GUID.
Populate it during pseudo probe section parsing, and emit it in YAML
profile (both regular and BAT), along with function checksum.

To be used for stale function matching.

Test Plan: update pseudoprobe-decoding-inline.test

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Add a BinaryFunction field for pseudo probe function description hash.
Populate it during pseudo probe section parsing, and emit it in YAML
profile (both regular and BAT).

To be used for stale function matching.

Test Plan: update pseudoprobe-decoding-inline.test

---

Full diff: https://github.com/llvm/llvm-project/pull/99389.diff

6 Files Affected:

• (modified) bolt/include/bolt/Core/BinaryFunction.h (+9)
• (modified) bolt/include/bolt/Profile/ProfileYAMLMapping.h (+2)
• (modified) bolt/lib/Profile/DataAggregator.cpp (+1)
• (modified) bolt/lib/Profile/YAMLProfileWriter.cpp (+1)
• (modified) bolt/lib/Rewrite/PseudoProbeRewriter.cpp (+13-1)
• (modified) bolt/test/X86/pseudoprobe-decoding-inline.test (+13)

diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 7d0afee4d1b78..7e2f0bebebe03 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -416,6 +416,10 @@ class BinaryFunction {
   /// different parameters by every pass.
   mutable uint64_t Hash{0};
 
+  /// Function hash assigned by the compiler and stored in pseudo probe
+  /// description of the source function.
+  uint64_t PseudoProbeDescHash{0};
+
   /// For PLT functions it contains a symbol associated with a function
   /// reference. It is nullptr for non-PLT functions.
   const MCSymbol *PLTSymbol{nullptr};
@@ -2256,6 +2260,11 @@ class BinaryFunction {
   /// Returns the last computed hash value of the function.
   size_t getHash() const { return Hash; }
 
+  /// Returns the function hash from pseudo-probe description of the function.
+  uint64_t getPseudoProbeDescHash() const { return PseudoProbeDescHash; }
+
+  void setPseudoProbeDescHash(uint64_t Hash) { PseudoProbeDescHash = Hash; }
+
   using OperandHashFuncTy =
       function_ref<typename std::string(const MCOperand &)>;
 
diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
index 9dd3920dbf094..1c752a990c099 100644
--- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
+++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
@@ -151,6 +151,7 @@ struct BinaryFunctionProfile {
   llvm::yaml::Hex64 Hash{0};
   uint64_t ExecCount{0};
   std::vector<BinaryBasicBlockProfile> Blocks;
+  llvm::yaml::Hex64 PseudoProbeDescHash{0};
   bool Used{false};
 };
 } // end namespace bolt
@@ -164,6 +165,7 @@ template <> struct MappingTraits<bolt::BinaryFunctionProfile> {
     YamlIO.mapRequired("nblocks", BFP.NumBasicBlocks);
     YamlIO.mapOptional("blocks", BFP.Blocks,
                        std::vector<bolt::BinaryBasicBlockProfile>());
+    YamlIO.mapOptional("pseudo_probe_desc_hash", BFP.PseudoProbeDescHash);
   }
 };
 
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index ce6ec0a04ac16..94463980a738a 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2345,6 +2345,7 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
       YamlBF.Hash = BAT->getBFHash(FuncAddress);
       YamlBF.ExecCount = BF->getKnownExecutionCount();
       YamlBF.NumBasicBlocks = BAT->getNumBasicBlocks(FuncAddress);
+      YamlBF.PseudoProbeDescHash = BF->getPseudoProbeDescHash();
       const BoltAddressTranslation::BBHashMapTy &BlockMap =
           BAT->getBBHashMap(FuncAddress);
       YamlBF.Blocks.resize(YamlBF.NumBasicBlocks);
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 9adbfdc5ff089..e7a8239e6b20a 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -69,6 +69,7 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
   YamlBF.Hash = BF.getHash();
   YamlBF.NumBasicBlocks = BF.size();
   YamlBF.ExecCount = BF.getKnownExecutionCount();
+  YamlBF.PseudoProbeDescHash = BF.getPseudoProbeDescHash();
 
   BinaryFunction::BasicBlockOrderType Order;
   llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
index 51038dbead330..67219b658f527 100644
--- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
+++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp
@@ -78,11 +78,17 @@ class PseudoProbeRewriter final : public MetadataRewriter {
   PseudoProbeRewriter(BinaryContext &BC)
       : MetadataRewriter("pseudo-probe-rewriter", BC) {}
 
+  Error preCFGInitializer() override;
   Error postEmitFinalizer() override;
 };
 
-Error PseudoProbeRewriter::postEmitFinalizer() {
+Error PseudoProbeRewriter::preCFGInitializer() {
   parsePseudoProbe();
+
+  return Error::success();
+}
+
+Error PseudoProbeRewriter::postEmitFinalizer() {
   updatePseudoProbes();
 
   return Error::success();
@@ -138,6 +144,12 @@ void PseudoProbeRewriter::parsePseudoProbe() {
     ProbeDecoder.printGUID2FuncDescMap(outs());
     ProbeDecoder.printProbesForAllAddresses(outs());
   }
+
+  for (const auto &[GUID, FuncDesc] : ProbeDecoder.getGUID2FuncDescMap())
+    if (FuncStartAddrs.contains(GUID))
+      if (BinaryFunction *BF =
+              BC.getBinaryFunctionAtAddress(FuncStartAddrs[GUID]))
+        BF->setPseudoProbeDescHash(FuncDesc.FuncHash);
 }
 
 void PseudoProbeRewriter::updatePseudoProbes() {
diff --git a/bolt/test/X86/pseudoprobe-decoding-inline.test b/bolt/test/X86/pseudoprobe-decoding-inline.test
index 15e93b1630a66..af9b96764908e 100644
--- a/bolt/test/X86/pseudoprobe-decoding-inline.test
+++ b/bolt/test/X86/pseudoprobe-decoding-inline.test
@@ -1,6 +1,19 @@
 # REQUIRES: system-linux
 # RUN: llvm-bolt  %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin --print-pseudo-probes=all -o %t.bolt 2>&1 | FileCheck %s
 
+# PREAGG: B X:0 #foo# 1 0
+# PREAGG: B X:0 #bar# 1 0
+# PREAGG: B X:0 #main# 1 0
+# RUN: link_fdata %s %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin %t.preagg PREAGG
+# RUN: perf2bolt %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin -p %t.preagg --pa -w %t.yaml -o %t.fdata
+# RUN: FileCheck --input-file %t.yaml %s --check-prefix CHECK-YAML
+# CHECK-YAML: name: bar
+# CHECK-YAML: pseudo_probe_desc_hash: 0x10E852DA94
+# CHECK-YAML: name: foo
+# CHECK-YAML: pseudo_probe_desc_hash: 0x200205A19C5B4
+# CHECK-YAML: name: main
+# CHECK-YAML: pseudo_probe_desc_hash: 0x10000FFFFFFFF
+
 CHECK: Report of decoding input pseudo probe binaries
 
 CHECK-NEXT: Pseudo Probe Desc:

[wlei-llvm]

LGTM.

[WenleiHe]

In theory we just need a function checksum, plus a set of stable block Ids for each function. I think we don't have to prefix everything with "pseudo-probe" -- probe is one way, and the current way of getting checksum and stable block id for llvm compiled binary when csspgo is used, but at bolt level, it doesn't have to be tied to probe and it can be anything that the compiler and binary provides.

So the APIs for yaml and BF should operate at level with "checksum" and "blockId" abstraction, while the current underlying implementation provides these info through what we retrieve from probe descriptor.

[aaupov]

@WenleiHe: addressed your comments. Please let me know if the current state is good to go.

[WenleiHe]

This isn't really a pseudo-probe hash, just like GUID isn't really a pseudo-probe GUID. Can we call it CFG checksum/hash?

[WenleiHe]

@WenleiHe: addressed your comments. Please let me know if the current state is good to go.

@aaupov one more comment related to what I mentioned above.