Revert "[BOLT] Hash-based function matching"

[shawbyoung]

Reverts #95821

[llvmbot]

@llvm/pr-subscribers-bolt

Author: shaw young (shawbyoung)

Changes

Reverts llvm/llvm-project#95821

---

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

5 Files Affected:

• (modified) bolt/docs/CommandLineArgumentReference.md (-4)
• (modified) bolt/lib/Profile/YAMLProfileReader.cpp (+10-58)
• (modified) bolt/lib/Rewrite/RewriteInstance.cpp (-4)
• (modified) bolt/lib/Utils/CommandLineOpts.cpp (-5)
• (removed) bolt/test/X86/hashing-based-function-matching.test (-64)

diff --git a/bolt/docs/CommandLineArgumentReference.md b/bolt/docs/CommandLineArgumentReference.md
index 00d472c578916..d95f30a299a28 100644
--- a/bolt/docs/CommandLineArgumentReference.md
+++ b/bolt/docs/CommandLineArgumentReference.md
@@ -259,10 +259,6 @@
 
   Always use long jumps/nops for Linux kernel static keys
 
-- `--match-profile-with-function-hash`
-
-  Match profile with function hash
-
 - `--max-data-relocations=<uint>`
 
   Maximum number of data relocations to process
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 6c4eece4ddb1b..f25f59201f1cd 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -22,8 +22,6 @@ namespace opts {
 extern cl::opt<unsigned> Verbosity;
 extern cl::OptionCategory BoltOptCategory;
 extern cl::opt<bool> InferStaleProfile;
-extern cl::opt<bool> MatchProfileWithFunctionHash;
-extern cl::opt<bool> Lite;
 
 static llvm::cl::opt<bool>
     IgnoreHash("profile-ignore-hash",
@@ -365,19 +363,9 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
     return Profile.Hash == static_cast<uint64_t>(BF.getHash());
   };
 
-  uint64_t MatchedWithExactName = 0;
-  uint64_t MatchedWithHash = 0;
-  uint64_t MatchedWithLTOCommonName = 0;
-
-  // Computes hash for binary functions.
-  if (opts::MatchProfileWithFunctionHash)
-    for (auto &[_, BF] : BC.getBinaryFunctions())
-      BF.computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
-  else if (!opts::IgnoreHash)
-    for (BinaryFunction *BF : ProfileBFs)
-      BF->computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
-
-  // This first pass assigns profiles that match 100% by name and by hash.
+  // We have to do 2 passes since LTO introduces an ambiguity in function
+  // names. The first pass assigns profiles that match 100% by name and
+  // by hash. The second pass allows name ambiguity for LTO private functions.
   for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs)) {
     if (!BF)
       continue;
@@ -386,34 +374,15 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
     // the profile.
     Function.setExecutionCount(BinaryFunction::COUNT_NO_PROFILE);
 
-    if (profileMatches(YamlBF, Function)) {
-      matchProfileToFunction(YamlBF, Function);
-      ++MatchedWithExactName;
-    }
-  }
-
-  // Uses the strict hash of profiled and binary functions to match functions
-  // that are not matched by name or common name.
-  if (opts::MatchProfileWithFunctionHash) {
-    std::unordered_map<size_t, BinaryFunction *> StrictHashToBF;
-    StrictHashToBF.reserve(BC.getBinaryFunctions().size());
+    // Recompute hash once per function.
+    if (!opts::IgnoreHash)
+      Function.computeHash(YamlBP.Header.IsDFSOrder,
+                           YamlBP.Header.HashFunction);
 
-    for (auto &[_, BF] : BC.getBinaryFunctions())
-      StrictHashToBF[BF.getHash()] = &BF;
-
-    for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
-      if (YamlBF.Used)
-        continue;
-      auto It = StrictHashToBF.find(YamlBF.Hash);
-      if (It != StrictHashToBF.end() && !ProfiledFunctions.count(It->second)) {
-        BinaryFunction *BF = It->second;
-        matchProfileToFunction(YamlBF, *BF);
-        ++MatchedWithHash;
-      }
-    }
+    if (profileMatches(YamlBF, Function))
+      matchProfileToFunction(YamlBF, Function);
   }
 
-  // This second pass allows name ambiguity for LTO private functions.
   for (const auto &[CommonName, LTOProfiles] : LTOCommonNameMap) {
     if (!LTOCommonNameFunctionMap.contains(CommonName))
       continue;
@@ -427,7 +396,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
       for (BinaryFunction *BF : Functions) {
         if (!ProfiledFunctions.count(BF) && profileMatches(*YamlBF, *BF)) {
           matchProfileToFunction(*YamlBF, *BF);
-          ++MatchedWithLTOCommonName;
           return true;
         }
       }
@@ -439,10 +407,8 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
     // partially.
     if (!ProfileMatched && LTOProfiles.size() == 1 && Functions.size() == 1 &&
         !LTOProfiles.front()->Used &&
-        !ProfiledFunctions.count(*Functions.begin())) {
+        !ProfiledFunctions.count(*Functions.begin()))
       matchProfileToFunction(*LTOProfiles.front(), **Functions.begin());
-      ++MatchedWithLTOCommonName;
-    }
   }
 
   for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs))
@@ -454,15 +420,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
       errs() << "BOLT-WARNING: profile ignored for function " << YamlBF.Name
              << '\n';
 
-  if (opts::Verbosity >= 2) {
-    outs() << "BOLT-INFO: matched " << MatchedWithExactName
-           << " functions with identical names\n";
-    outs() << "BOLT-INFO: matched " << MatchedWithHash
-           << " functions with hash\n";
-    outs() << "BOLT-INFO: matched " << MatchedWithLTOCommonName
-           << " functions with matching LTO common names\n";
-  }
-
   // Set for parseFunctionProfile().
   NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions");
   NormalizeByCalls = usesEvent("branches");
@@ -482,11 +439,6 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
 
   BC.setNumUnusedProfiledObjects(NumUnused);
 
-  if (opts::Lite)
-    for (BinaryFunction *BF : BC.getAllBinaryFunctions())
-      if (!BF->hasProfile())
-        BF->setIgnored();
-
   return Error::success();
 }
 
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index ec234add8a6f5..1a3a8af21d81b 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -82,7 +82,6 @@ extern cl::opt<bool> Hugify;
 extern cl::opt<bool> Instrument;
 extern cl::opt<JumpTableSupportLevel> JumpTables;
 extern cl::opt<bool> KeepNops;
-extern cl::opt<bool> MatchProfileWithFunctionHash;
 extern cl::list<std::string> ReorderData;
 extern cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions;
 extern cl::opt<bool> TerminalTrap;
@@ -2983,9 +2982,6 @@ void RewriteInstance::selectFunctionsToProcess() {
     if (mustSkip(Function))
       return false;
 
-    if (opts::MatchProfileWithFunctionHash)
-      return true;
-
     // If the list is not empty, only process functions from the list.
     if (!opts::ForceFunctionNames.empty() || !ForceFunctionsNR.empty()) {
       // Regex check (-funcs and -funcs-file options).
diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp
index 0724e627dfd19..41c89bc8aeba4 100644
--- a/bolt/lib/Utils/CommandLineOpts.cpp
+++ b/bolt/lib/Utils/CommandLineOpts.cpp
@@ -128,11 +128,6 @@ cl::opt<bool>
                cl::desc("instrument code to generate accurate profile data"),
                cl::cat(BoltOptCategory));
 
-cl::opt<bool>
-    MatchProfileWithFunctionHash("match-profile-with-function-hash",
-                                 cl::desc("Match profile with function hash"),
-                                 cl::Hidden, cl::cat(BoltCategory));
-
 cl::opt<std::string>
 OutputFilename("o",
   cl::desc("<output file>"),
diff --git a/bolt/test/X86/hashing-based-function-matching.test b/bolt/test/X86/hashing-based-function-matching.test
deleted file mode 100644
index 4426da085bbd9..0000000000000
--- a/bolt/test/X86/hashing-based-function-matching.test
+++ /dev/null
@@ -1,64 +0,0 @@
-## Tests function matching in YAMLProfileReader by function hash.
-
-# REQUIRES: system-linux
-# RUN: split-file %s %t
-# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %t/main.s -o %t.o
-# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
-# RUN: llvm-bolt %t.exe -o %t.out --data %t/yaml -v=2 \
-# RUN:   --print-cfg --match-profile-with-function-hash 2>&1 --profile-ignore-hash=0 | FileCheck %s
-
-# CHECK: BOLT-INFO: matched 1 functions with hash
-
-#--- main.s
-.globl main
-.type	main, @function
-main:
-  .cfi_startproc
-.LBB00:
-  pushq   %rbp
-  movq    %rsp, %rbp
-  subq    $16, %rsp
-  testq   %rax, %rax
-  js      .LBB03
-.LBB01:
-  jne     .LBB04
-.LBB02:
-  nop
-.LBB03:
-  xorl    %eax, %eax
-  addq    $16, %rsp
-  popq    %rbp
-  retq
-.LBB04:
-  xorl    %eax, %eax
-  addq    $16, %rsp
-  popq    %rbp
-  retq
-## For relocations against .text
-.LBB05:
-  call exit
-  .cfi_endproc
-  .size	main, .-main
-
-#--- yaml
----
-header:
-  profile-version: 1
-  binary-name:     'hashing-based-function-matching.s.tmp.exe'
-  binary-build-id: '<unknown>'
-  profile-flags:   [ lbr ]
-  profile-origin:  branch profile reader
-  profile-events:  ''
-  dfs-order:       false
-  hash-func:       xxh3
-functions:
-  - name:            main2
-    fid:             0
-    hash:            0x72F82DEAA6FE65FB
-    exec:            1
-    nblocks:         6
-    blocks:
-      - bid:             1
-        insns:           1
-        succ:            [ { bid: 3, cnt: 1} ]
-...

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[nico]

(merged to fix build)