-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[BOLT] Hash-based function matching #96822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
shawbyoung
wants to merge
1
commit into
main
from
users/shawbyoung/spr/bolt-hash-based-function-matching-4
Closed
[BOLT] Hash-based function matching #96822
shawbyoung
wants to merge
1
commit into
main
from
users/shawbyoung/spr/bolt-hash-based-function-matching-4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.4
@llvm/pr-subscribers-bolt Author: shaw young (shawbyoung) ChangesTest Plan: Full diff: https://github.com/llvm/llvm-project/pull/96822.diff 6 Files Affected:
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..0a2f068c53c2b 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,21 @@ 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) {
+ 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)
continue;
@@ -374,15 +388,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 +429,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 +441,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 +456,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 +484,11 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
BC.setNumUnusedProfiledObjects(NumUnused);
+ if (opts::Lite && opts::MatchProfileWithFunctionHash)
+ 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..ee6ac1c7d57b5 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -82,6 +82,8 @@ 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;
extern cl::opt<bool> TerminalTrap;
@@ -140,9 +142,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 +2981,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} ]
+...
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 416b3952f1ac4..bc7263c53ba61 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -367,6 +367,12 @@ Changes to the LLVM tools
Changes to LLDB
---------------------------------
+Changes to BOLT
+---------------------------------
+* Now supports ``--match-profile-with-function-hash`` to match profiled and
+ binary functions with exact hash, allowing for the matching of renamed but
+ identical functions.
+
Changes to Sanitizers
---------------------
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test Plan:
added
hashing-based-function-matching.test.