[llvm] Revert "[BOLT] Hash-based function matching" (PR #96568)
shaw young via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 24 15:34:49 PDT 2024
https://github.com/shawbyoung created https://github.com/llvm/llvm-project/pull/96568
Reverts llvm/llvm-project#95821
>From acb7a1bd6b25c4c9f9d9a7c86e36df759b887e90 Mon Sep 17 00:00:00 2001
From: shaw young <58664393+shawbyoung at users.noreply.github.com>
Date: Mon, 24 Jun 2024 15:34:05 -0700
Subject: [PATCH] Revert "[BOLT] Hash-based function matching (#95821)"
This reverts commit 5e097c79d820683b72e2bac8e56df93801ad85ec.
---
bolt/docs/CommandLineArgumentReference.md | 4 --
bolt/lib/Profile/YAMLProfileReader.cpp | 68 +++----------------
bolt/lib/Rewrite/RewriteInstance.cpp | 4 --
bolt/lib/Utils/CommandLineOpts.cpp | 5 --
.../X86/hashing-based-function-matching.test | 64 -----------------
5 files changed, 10 insertions(+), 135 deletions(-)
delete mode 100644 bolt/test/X86/hashing-based-function-matching.test
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} ]
-...
More information about the llvm-commits
mailing list