[llvm] [BOLT] Introduce profile-use-std-hash (PR #74253)

Amir Ayupov via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 3 13:26:08 PST 2023


https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/74253

Provide backwards compatibility for YAML profile that uses `std::hash`:
xxh3 hash is the default for newly produced profile (sets `std-hash: false`),
whereas the profile that doesn't specify `std-hash` will be treated as
`std-hash: true`, preserving old behavior.

Same as `profile-use-dfs` in https://reviews.llvm.org/D156176.


>From 1ded354afb256fff255871696333e9e9fad3f0a3 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Sun, 3 Dec 2023 13:08:28 -0800
Subject: [PATCH] [BOLT] Introduce profile-use-std-hash

Provide backwards compatibility for YAML profile that uses `std::hash`:
xxh3 hash is the default for newly produced profile (sets `std-hash: false`),
whereas the profile that doesn't specify `std-hash` will be treated as
`std-hash: true`, preserving old behavior.

Same as `profile-use-dfs` in https://reviews.llvm.org/D156176.
---
 bolt/include/bolt/Core/BinaryFunction.h       |  8 +++++++-
 .../include/bolt/Profile/ProfileYAMLMapping.h |  2 ++
 bolt/lib/Core/BinaryFunction.cpp              |  4 ++--
 bolt/lib/Passes/IdenticalCodeFolding.cpp      |  2 +-
 bolt/lib/Profile/StaleProfileMatching.cpp     |  6 +++---
 bolt/lib/Profile/YAMLProfileReader.cpp        | 10 ++++++++--
 bolt/lib/Profile/YAMLProfileWriter.cpp        |  5 +++--
 bolt/test/X86/pre-aggregated-perf.test        | 19 +++++++++++++++----
 8 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 182d5ff049c37..944755168c5c8 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -2234,18 +2234,24 @@ class BinaryFunction {
   ///
   /// If \p UseDFS is set, process basic blocks in DFS order. Otherwise, use
   /// the existing layout order.
+  /// If \p UseStdHash is set, use std::hash whose result might be
+  /// implementation-specific. Otherwise use xxhash.
   ///
   /// By default, instruction operands are ignored while calculating the hash.
   /// The caller can change this via passing \p OperandHashFunc function.
   /// The return result of this function will be mixed with internal hash.
   size_t computeHash(
       bool UseDFS = false,
+      bool UseStdHash = false,
       OperandHashFuncTy OperandHashFunc = [](const MCOperand &) {
         return std::string();
       }) const;
 
   /// Compute hash values for each block of the function.
-  void computeBlockHashes() const;
+  ///
+  /// If \p UseStdHash is set, use std::hash whose result might be
+  /// implementation-specific. Otherwise use xxhash.
+  void computeBlockHashes(bool UseStdHash = false) const;
 
   void setDWARFUnit(DWARFUnit *Unit) { DwarfUnit = Unit; }
 
diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
index 2218a167a74ec..54c3c50cc3d8a 100644
--- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
+++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
@@ -188,6 +188,7 @@ struct BinaryProfileHeader {
   std::string Origin;     // How the profile was obtained.
   std::string EventNames; // Events used for sample profile.
   bool IsDFSOrder{true};  // Whether using DFS block order in function profile
+  bool IsStdHash{true};   // Whether using std::hash in function hashing
 };
 } // end namespace bolt
 
@@ -200,6 +201,7 @@ template <> struct MappingTraits<bolt::BinaryProfileHeader> {
     YamlIO.mapOptional("profile-origin", Header.Origin);
     YamlIO.mapOptional("profile-events", Header.EventNames);
     YamlIO.mapOptional("dfs-order", Header.IsDFSOrder);
+    YamlIO.mapOptional("std-hash", Header.IsStdHash);
   }
 };
 
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index be033cf076688..ed350e06e752a 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -3633,7 +3633,7 @@ BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
   return DFS;
 }
 
-size_t BinaryFunction::computeHash(bool UseDFS,
+size_t BinaryFunction::computeHash(bool UseDFS, bool UseStdHash,
                                    OperandHashFuncTy OperandHashFunc) const {
   if (size() == 0)
     return 0;
@@ -3652,7 +3652,7 @@ size_t BinaryFunction::computeHash(bool UseDFS,
   for (const BinaryBasicBlock *BB : Order)
     HashString.append(hashBlock(BC, *BB, OperandHashFunc));
 
-  return Hash = llvm::xxh3_64bits(HashString);
+  return Hash = UseStdHash ? std::hash<std::string>{}(HashString) : llvm::xxh3_64bits(HashString);
 }
 
 void BinaryFunction::insertBasicBlocks(
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index b4ec89ca8fd79..7d6ed6f807626 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -360,7 +360,7 @@ void IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
 
       // Pre-compute hash before pushing into hashtable.
       // Hash instruction operands to minimize hash collisions.
-      BF.computeHash(opts::ICFUseDFS, [&BC](const MCOperand &Op) {
+      BF.computeHash(opts::ICFUseDFS, /*UseStdHash=*/false, [&BC](const MCOperand &Op) {
         return hashInstOperand(BC, Op);
       });
     };
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 6fb6f380f71ee..3f027bb4ba498 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -225,7 +225,7 @@ class StaleMatcher {
   std::unordered_map<uint16_t, std::vector<HashBlockPairType>> OpHashToBlocks;
 };
 
-void BinaryFunction::computeBlockHashes() const {
+void BinaryFunction::computeBlockHashes(bool UseStdHash) const {
   if (size() == 0)
     return;
 
@@ -241,11 +241,11 @@ void BinaryFunction::computeBlockHashes() const {
     // Hashing complete instructions.
     std::string InstrHashStr = hashBlock(
         BC, *BB, [&](const MCOperand &Op) { return hashInstOperand(BC, Op); });
-    uint64_t InstrHash = llvm::xxh3_64bits(InstrHashStr);
+    uint64_t InstrHash = UseStdHash ? std::hash<std::string>{}(InstrHashStr) : llvm::xxh3_64bits(InstrHashStr);
     BlendedHashes[I].InstrHash = (uint16_t)InstrHash;
     // Hashing opcodes.
     std::string OpcodeHashStr = hashBlockLoose(BC, *BB);
-    OpcodeHashes[I] = llvm::xxh3_64bits(OpcodeHashStr);
+    OpcodeHashes[I] = UseStdHash ? std::hash<std::string>{}(OpcodeHashStr) : llvm::xxh3_64bits(OpcodeHashStr);
     BlendedHashes[I].OpcodeHash = (uint16_t)OpcodeHashes[I];
   }
 
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 079cb352d36e7..9ae52f3df6dce 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -31,6 +31,11 @@ static llvm::cl::opt<bool>
 llvm::cl::opt<bool> ProfileUseDFS("profile-use-dfs",
                                   cl::desc("use DFS order for YAML profile"),
                                   cl::Hidden, cl::cat(BoltOptCategory));
+
+llvm::cl::opt<bool>
+    ProfileUseStdHash("profile-use-std-hash",
+                      cl::desc("use std::hash for YAML profile"), cl::Hidden,
+                      cl::cat(BoltOptCategory));
 } // namespace opts
 
 namespace llvm {
@@ -83,6 +88,7 @@ bool YAMLProfileReader::parseFunctionProfile(
   BinaryContext &BC = BF.getBinaryContext();
 
   const bool IsDFSOrder = YamlBP.Header.IsDFSOrder;
+  const bool IsStdHash = YamlBP.Header.IsStdHash;
   bool ProfileMatched = true;
   uint64_t MismatchedBlocks = 0;
   uint64_t MismatchedCalls = 0;
@@ -98,7 +104,7 @@ bool YAMLProfileReader::parseFunctionProfile(
       FuncRawBranchCount += YamlSI.Count;
   BF.setRawBranchCount(FuncRawBranchCount);
 
-  if (!opts::IgnoreHash && YamlBF.Hash != BF.computeHash(IsDFSOrder)) {
+  if (!opts::IgnoreHash && YamlBF.Hash != BF.computeHash(IsDFSOrder, IsStdHash)) {
     if (opts::Verbosity >= 1)
       errs() << "BOLT-WARNING: function hash mismatch\n";
     ProfileMatched = false;
@@ -348,7 +354,7 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
 
     // Recompute hash once per function.
     if (!opts::IgnoreHash)
-      Function.computeHash(YamlBP.Header.IsDFSOrder);
+      Function.computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.IsStdHash);
 
     if (profileMatches(YamlBF, Function))
       matchProfileToFunction(YamlBF, Function);
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 3326d1d8f5596..bb0b695f07d60 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -21,6 +21,7 @@
 
 namespace opts {
 extern llvm::cl::opt<bool> ProfileUseDFS;
+extern llvm::cl::opt<bool> ProfileUseStdHash;
 } // namespace opts
 
 namespace llvm {
@@ -34,8 +35,8 @@ void convert(const BinaryFunction &BF,
   const uint16_t LBRProfile = BF.getProfileFlags() & BinaryFunction::PF_LBR;
 
   // Prepare function and block hashes
-  BF.computeHash(opts::ProfileUseDFS);
-  BF.computeBlockHashes();
+  BF.computeHash(opts::ProfileUseDFS, opts::ProfileUseStdHash);
+  BF.computeBlockHashes(opts::ProfileUseStdHash);
 
   YamlBF.Name = BF.getPrintName();
   YamlBF.Id = BF.getFunctionNumber();
diff --git a/bolt/test/X86/pre-aggregated-perf.test b/bolt/test/X86/pre-aggregated-perf.test
index e8c3f64239a27..4fb02c58d2d27 100644
--- a/bolt/test/X86/pre-aggregated-perf.test
+++ b/bolt/test/X86/pre-aggregated-perf.test
@@ -13,7 +13,10 @@ RUN: yaml2obj %p/Inputs/blarge.yaml &> %t.exe
 RUN: perf2bolt %t.exe -o %t --pa -p %p/Inputs/pre-aggregated.txt -w %t.new \
 RUN:   --profile-use-dfs
 RUN: cat %t | sort | FileCheck %s -check-prefix=PERF2BOLT
-RUN: cat %t.new | FileCheck %s -check-prefix=NEWFORMAT
+RUN: cat %t.new | FileCheck %s -check-prefix=NEWFORMAT -DHASH=0x28C72085C0BD8D37
+RUN: perf2bolt %t.exe -o %t --pa -p %p/Inputs/pre-aggregated.txt -w %t.new \
+RUN:   --profile-use-dfs --profile-use-std-hash
+RUN: cat %t.new | FileCheck %s -check-prefix=NEWFORMAT -DHASH=0x24496F7F9594E89F
 
 # Test --profile-format option with perf2bolt
 RUN: perf2bolt %t.exe -o %t.fdata --pa -p %p/Inputs/pre-aggregated.txt \
@@ -22,7 +25,11 @@ RUN: cat %t.fdata | sort | FileCheck %s -check-prefix=PERF2BOLT
 
 RUN: perf2bolt %t.exe -o %t.yaml --pa -p %p/Inputs/pre-aggregated.txt \
 RUN:   --profile-format=yaml --profile-use-dfs
-RUN: cat %t.yaml | FileCheck %s -check-prefix=NEWFORMAT
+RUN: cat %t.yaml | FileCheck %s -check-prefix=NEWFORMAT -DHASH=0x28C72085C0BD8D37
+
+RUN: perf2bolt %t.exe -o %t.yaml --pa -p %p/Inputs/pre-aggregated.txt \
+RUN:   --profile-format=yaml --profile-use-dfs --profile-use-std-hash
+RUN: cat %t.yaml | FileCheck %s -check-prefix=NEWFORMAT -DHASH=0x24496F7F9594E89F
 
 # Test --profile-format option with llvm-bolt --aggregate-only
 RUN: llvm-bolt %t.exe -o %t.bolt.fdata --pa -p %p/Inputs/pre-aggregated.txt \
@@ -31,7 +38,11 @@ RUN: cat %t.bolt.fdata | sort | FileCheck %s -check-prefix=PERF2BOLT
 
 RUN: llvm-bolt %t.exe -o %t.bolt.yaml --pa -p %p/Inputs/pre-aggregated.txt \
 RUN:   --aggregate-only --profile-format=yaml --profile-use-dfs
-RUN: cat %t.bolt.yaml | FileCheck %s -check-prefix=NEWFORMAT
+RUN: cat %t.bolt.yaml | FileCheck %s -check-prefix=NEWFORMAT -DHASH=0x28C72085C0BD8D37
+
+RUN: llvm-bolt %t.exe -o %t.bolt.yaml --pa -p %p/Inputs/pre-aggregated.txt \
+RUN:   --aggregate-only --profile-format=yaml --profile-use-dfs --profile-use-std-hash
+RUN: cat %t.bolt.yaml | FileCheck %s -check-prefix=NEWFORMAT -DHASH=0x24496F7F9594E89F
 
 PERF2BOLT: 0 [unknown] 7f36d18d60c0 1 main 53c 0 2
 PERF2BOLT: 1 main 451 1 SolveCubic 0 0 2
@@ -46,7 +57,7 @@ PERF2BOLT: 1 usqrt a 1 usqrt 10 0 22
 
 NEWFORMAT:  - name:            'frame_dummy/1'
 NEWFORMAT:    fid:             3
-NEWFORMAT:    hash:            0x28C72085C0BD8D37
+NEWFORMAT:    hash:            [[HASH]]
 NEWFORMAT:    exec:            1
 
 NEWFORMAT:  - name:            usqrt



More information about the llvm-commits mailing list