-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[BOLT] Discard BB profiles with a hash of 0 in yaml from a Post-BAT binary #169627
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
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-bolt Author: Jinjie Huang (Jinjie-Huang) ChangesWhen generating the YAML profile from a Post-BAT binary, the writer seems to unconditionally trust and use the BBHashMap from the BAT table. However, it appears that in some scenarios, a recorded basic block (BB) hash within this map is 0. When infer-stale-profile is enabled, profile.yaml generated this way may trigger an assertion failure here and crash. So this patch tries to drop the BB profiles with a hash of 0 in writeBATYAML(). Full diff: https://github.com/llvm/llvm-project/pull/169627.diff 1 Files Affected:
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 6b969011df589..dcda502a6912d 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2417,6 +2417,7 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
// Skip printing if there's no profile data
llvm::erase_if(
YamlBF.Blocks, [](const yaml::bolt::BinaryBasicBlockProfile &YamlBB) {
+ if ((size_t)YamlBB.Hash == 0) return true;
auto HasCount = [](const auto &SI) { return SI.Count; };
bool HasAnyCount = YamlBB.ExecCount ||
llvm::any_of(YamlBB.Successors, HasCount) ||
|
|
The BAT entry dump from the case we encountered in practice: And the corresponding profile.yaml: - name: 'Curl_close/url.c/1'
fid: 2418089
hash: 0x1DCC63912D34940D
exec: 9
nblocks: 14
blocks:
- bid: 0
insns: 0
hash: 0x2600D303C7740000
exec: 9
succ: [ { bid: 1, cnt: 9 } ]
- bid: 1
insns: 0
hash: 0x204210DFB8D0000E
succ: [ { bid: 2, cnt: 1 }, { bid: 13, cnt: 8, mis: 2 } ]
- bid: 2
insns: 0
hash: 0xDDC0155A5E1B001A
calls: [ { off: 0xA, fid: 2418075, cnt: 1 } ]
succ: [ { bid: 4, cnt: 1 } ]
- bid: 4
insns: 0
hash: 0xDD56DA00B8D0003D
succ: [ { bid: 6, cnt: 1, mis: 1 } ]
- bid: 5
insns: 0
hash: 0x69C0E100F2CA0049
succ: [ { bid: 6, cnt: 1 } ]
- bid: 6
insns: 0
hash: 0x74DD12BA8D7F0059
calls: [ { off: 0x9, fid: 2418070, cnt: 2 } ]
succ: [ { bid: 8, cnt: 2, mis: 1 } ]
- bid: 8
insns: 0
hash: 0x3D72E8558D5008A
calls: [ { off: 0x3, fid: 2412008, cnt: 2 }, { off: 0xB, fid: 2412673, cnt: 2 } ]
**- bid: 0
insns: 0
calls: [ { off: 0x37, fid: 2412928, cnt: 2, mis: 1 } ]
succ: [ { bid: 13, cnt: 2 } ]**Alternatively, we may handle this in inferStaleProfile() by having it detect this scenario and not trigger an assertion failure for such BBs. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
7d7c947 to
f9bb4b7
Compare
|
Thank you for the report. I have the proper fix, just need to put up a test case reproing the behavior. Please hold off with this diff, I'll put it up shortly. |
|
#145124 is the fix |
When generating the YAML profile from a Post-BAT binary, the writer seems to unconditionally trust and use the BBHashMap from the BAT table. However, it appears that in some scenarios, a recorded basic block (BB) hash within this map is 0.
When infer-stale-profile is enabled, profile.yaml generated by this way may trigger an assertion failure here and crash. So this patch tries to drop the BB profiles with a hash of 0 in writeBATYAML().