[llvm] [BOLT] Hash-based function matching (PR #96572)
shaw young via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 28 12:21:16 PDT 2024
https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/96572
>From e28dcc06eca4ea254d143ae8d1e9c698805e0d11 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 24 Jun 2024 15:51:24 -0700
Subject: [PATCH 1/7] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
---
bolt/docs/CommandLineArgumentReference.md | 4 ++
bolt/lib/Profile/YAMLProfileReader.cpp | 68 ++++++++++++++++---
bolt/lib/Rewrite/RewriteInstance.cpp | 7 +-
bolt/lib/Utils/CommandLineOpts.cpp | 8 +++
.../X86/hashing-based-function-matching.test | 64 +++++++++++++++++
5 files changed, 138 insertions(+), 13 deletions(-)
create mode 100644 bolt/test/X86/hashing-based-function-matching.test
diff --git a/bolt/docs/CommandLineArgumentReference.md b/bolt/docs/CommandLineArgumentReference.md
index d95f30a299a28..00d472c578916 100644
--- a/bolt/docs/CommandLineArgumentReference.md
+++ b/bolt/docs/CommandLineArgumentReference.md
@@ -259,6 +259,10 @@
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 f25f59201f1cd..6c4eece4ddb1b 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -22,6 +22,8 @@ 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",
@@ -363,9 +365,19 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
return Profile.Hash == static_cast<uint64_t>(BF.getHash());
};
- // 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.
+ 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.
for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs)) {
if (!BF)
continue;
@@ -374,15 +386,34 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
// the profile.
Function.setExecutionCount(BinaryFunction::COUNT_NO_PROFILE);
- // Recompute hash once per function.
- if (!opts::IgnoreHash)
- Function.computeHash(YamlBP.Header.IsDFSOrder,
- YamlBP.Header.HashFunction);
-
- if (profileMatches(YamlBF, Function))
+ 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());
+
+ 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;
+ }
+ }
+ }
+
+ // This second pass allows name ambiguity for LTO private functions.
for (const auto &[CommonName, LTOProfiles] : LTOCommonNameMap) {
if (!LTOCommonNameFunctionMap.contains(CommonName))
continue;
@@ -396,6 +427,7 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
for (BinaryFunction *BF : Functions) {
if (!ProfiledFunctions.count(BF) && profileMatches(*YamlBF, *BF)) {
matchProfileToFunction(*YamlBF, *BF);
+ ++MatchedWithLTOCommonName;
return true;
}
}
@@ -407,8 +439,10 @@ 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))
@@ -420,6 +454,15 @@ 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");
@@ -439,6 +482,11 @@ 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 1a3a8af21d81b..c0873a58a8a6e 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -82,6 +82,7 @@ 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;
@@ -140,9 +141,6 @@ KeepTmp("keep-tmp",
cl::Hidden,
cl::cat(BoltCategory));
-cl::opt<bool> Lite("lite", cl::desc("skip processing of cold functions"),
- cl::cat(BoltCategory));
-
static cl::opt<unsigned>
LiteThresholdPct("lite-threshold-pct",
cl::desc("threshold (in percent) for selecting functions to process in lite "
@@ -2982,6 +2980,9 @@ 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 41c89bc8aeba4..41de30f3f566b 100644
--- a/bolt/lib/Utils/CommandLineOpts.cpp
+++ b/bolt/lib/Utils/CommandLineOpts.cpp
@@ -128,6 +128,14 @@ cl::opt<bool>
cl::desc("instrument code to generate accurate profile data"),
cl::cat(BoltOptCategory));
+cl::opt<bool> Lite("lite", cl::desc("skip processing of cold functions"),
+ cl::cat(BoltCategory));
+
+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
new file mode 100644
index 0000000000000..4426da085bbd9
--- /dev/null
+++ b/bolt/test/X86/hashing-based-function-matching.test
@@ -0,0 +1,64 @@
+## 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} ]
+...
>From 07bb605507ba96ebf6f908b958e8c7e512cae927 Mon Sep 17 00:00:00 2001
From: shaw young <58664393+shawbyoung at users.noreply.github.com>
Date: Tue, 25 Jun 2024 08:11:03 -0700
Subject: [PATCH 2/7] Added Lite to RewriteInstance.cpp
---
bolt/lib/Rewrite/RewriteInstance.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index c0873a58a8a6e..ee6ac1c7d57b5 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -82,6 +82,7 @@ extern cl::opt<bool> Hugify;
extern cl::opt<bool> Instrument;
extern cl::opt<JumpTableSupportLevel> JumpTables;
extern cl::opt<bool> KeepNops;
+extern cl::opt<bool> Lite;
extern cl::opt<bool> MatchProfileWithFunctionHash;
extern cl::list<std::string> ReorderData;
extern cl::opt<bolt::ReorderFunctions::ReorderType> ReorderFunctions;
>From 91b05ac7203087763680f9443b09adde9d8a32f2 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Wed, 26 Jun 2024 09:52:01 -0700
Subject: [PATCH 3/7] Added check for nullptr BFs in hash computation in
readProfile
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 6c4eece4ddb1b..8ec5fa7f82e10 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -374,9 +374,11 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
for (auto &[_, BF] : BC.getBinaryFunctions())
BF.computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
else if (!opts::IgnoreHash)
- for (BinaryFunction *BF : ProfileBFs)
+ for (BinaryFunction *BF : ProfileBFs) {
+ if (!BF)
+ continue;
BF->computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
-
+ }
// This first pass assigns profiles that match 100% by name and by hash.
for (auto [YamlBF, BF] : llvm::zip_equal(YamlBP.Functions, ProfileBFs)) {
if (!BF)
>From 07454983fdab35a1f4e4407cc0d1c995d9a6aae2 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Wed, 26 Jun 2024 13:45:30 -0700
Subject: [PATCH 4/7] Formatting
Created using spr 1.3.4
---
bolt/docs/CommandLineArgumentReference.md | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/bolt/docs/CommandLineArgumentReference.md b/bolt/docs/CommandLineArgumentReference.md
index d95f30a299a28..00d472c578916 100644
--- a/bolt/docs/CommandLineArgumentReference.md
+++ b/bolt/docs/CommandLineArgumentReference.md
@@ -259,6 +259,10 @@
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
>From 797604b7455193d1dc9123d460ab3b31a900d87b Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 28 Jun 2024 09:49:07 -0700
Subject: [PATCH 5/7] Formatting
Created using spr 1.3.4
---
bolt/docs/CommandLineArgumentReference.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/docs/CommandLineArgumentReference.md b/bolt/docs/CommandLineArgumentReference.md
index e8bcc9f0128f4..00d472c578916 100644
--- a/bolt/docs/CommandLineArgumentReference.md
+++ b/bolt/docs/CommandLineArgumentReference.md
@@ -1236,4 +1236,4 @@
- `--print-options`
- Print non-default options after command line parsing
+ Print non-default options after command line parsing
\ No newline at end of file
>From eb6f5f12f784b45faca37bce273befd1b98fe78a Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 28 Jun 2024 11:23:44 -0700
Subject: [PATCH 6/7] Moved flag check for processing functions
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 2 ++
bolt/lib/Rewrite/RewriteInstance.cpp | 3 ---
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index f6dee95ed806c..74c1876585066 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -331,6 +331,8 @@ Error YAMLProfileReader::preprocessProfile(BinaryContext &BC) {
}
bool YAMLProfileReader::mayHaveProfileData(const BinaryFunction &BF) {
+ if (opts::MatchProfileWithFunctionHash)
+ return true;
for (StringRef Name : BF.getNames())
if (ProfileFunctionNames.contains(Name))
return true;
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index ee6ac1c7d57b5..7f8038bf9ab44 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -2981,9 +2981,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).
>From dc124900dc3c22cb0bd394da40432ca4659530c9 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 28 Jun 2024 12:21:04 -0700
Subject: [PATCH 7/7] Moved flag definition, comments
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 14 +++++++++-----
bolt/lib/Utils/CommandLineOpts.cpp | 5 -----
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 74c1876585066..f3a103dede8fa 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -30,6 +30,11 @@ static llvm::cl::opt<bool>
cl::desc("ignore hash while reading function profile"),
cl::Hidden, cl::cat(BoltOptCategory));
+llvm::cl::opt<bool>
+ MatchProfileWithFunctionHash("match-profile-with-function-hash",
+ cl::desc("Match profile with function hash"),
+ cl::Hidden, cl::cat(BoltOptCategory));
+
llvm::cl::opt<bool> ProfileUseDFS("profile-use-dfs",
cl::desc("use DFS order for YAML profile"),
cl::Hidden, cl::cat(BoltOptCategory));
@@ -376,8 +381,7 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
for (auto &[_, BF] : BC.getBinaryFunctions()) {
BF.computeHash(YamlBP.Header.IsDFSOrder, YamlBP.Header.HashFunction);
}
- }
- else if (!opts::IgnoreHash) {
+ } else if (!opts::IgnoreHash) {
for (BinaryFunction *BF : ProfileBFs) {
if (!BF)
continue;
@@ -400,9 +404,9 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
}
}
- // Uses the strict hash of profiled and binary functions to match functions
- // that are not matched by name or common name. Collisions are possible in the
- // unlikely case where multiple functions share the same exact hash.
+ // Iterates through profiled functions to match the first binary function with
+ // the same exact hash. Serves to match identical, renamed functions.
+ // Collisions are possible where multiple functions share the same exact hash.
if (opts::MatchProfileWithFunctionHash) {
DenseMap<size_t, BinaryFunction *> StrictHashToBF;
StrictHashToBF.reserve(BC.getBinaryFunctions().size());
diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp
index 41de30f3f566b..b9bc79f408a6b 100644
--- a/bolt/lib/Utils/CommandLineOpts.cpp
+++ b/bolt/lib/Utils/CommandLineOpts.cpp
@@ -131,11 +131,6 @@ cl::opt<bool>
cl::opt<bool> Lite("lite", cl::desc("skip processing of cold functions"),
cl::cat(BoltCategory));
-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>"),
More information about the llvm-commits
mailing list