[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
Shaw Young via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Jul 23 11:37:24 PDT 2024
https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/99891
>From 0274f697376264c2d77816190f9a434f64e79089 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 22 Jul 2024 11:56:23 -0700
Subject: [PATCH 1/3] Changed assignment of profiles with pseudo probe index
Created using spr 1.3.4
---
bolt/lib/Profile/StaleProfileMatching.cpp | 85 +++++++++++++++----
.../X86/match-blocks-with-pseudo-probes.test | 25 ++----
2 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 4105f626fb5b6..c135ee5ff4837 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -195,11 +195,15 @@ class StaleMatcher {
void init(const std::vector<FlowBlock *> &Blocks,
const std::vector<BlendedBlockHash> &Hashes,
const std::vector<uint64_t> &CallHashes,
- std::optional<uint64_t> YamlBFGUID) {
+ const std::unordered_map<uint64_t,
+ std::vector<const MCDecodedPseudoProbe *>>
+ IndexToBinaryPseudoProbes,
+ const std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *>
+ BinaryPseudoProbeToBlock,
+ const uint64_t YamlBFGUID) {
assert(Blocks.size() == Hashes.size() &&
Hashes.size() == CallHashes.size() &&
"incorrect matcher initialization");
-
for (size_t I = 0; I < Blocks.size(); I++) {
FlowBlock *Block = Blocks[I];
uint16_t OpHash = Hashes[I].OpcodeHash;
@@ -209,6 +213,8 @@ class StaleMatcher {
std::make_pair(Hashes[I], Block));
this->Blocks.push_back(Block);
}
+ this->IndexToBinaryPseudoProbes = IndexToBinaryPseudoProbes;
+ this->BinaryPseudoProbeToBlock = BinaryPseudoProbeToBlock;
this->YamlBFGUID = YamlBFGUID;
}
@@ -234,10 +240,14 @@ class StaleMatcher {
using HashBlockPairType = std::pair<BlendedBlockHash, FlowBlock *>;
std::unordered_map<uint16_t, std::vector<HashBlockPairType>> OpHashToBlocks;
std::unordered_map<uint64_t, std::vector<HashBlockPairType>> CallHashToBlocks;
- std::vector<FlowBlock *> Blocks;
+ std::unordered_map<uint64_t, std::vector<const MCDecodedPseudoProbe *>>
+ IndexToBinaryPseudoProbes;
+ std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *>
+ BinaryPseudoProbeToBlock;
+ std::vector<const FlowBlock *> Blocks;
// If the pseudo probe checksums of the profiled and binary functions are
// equal, then the YamlBF's GUID is defined and used to match blocks.
- std::optional<uint64_t> YamlBFGUID;
+ uint64_t YamlBFGUID;
// Uses OpcodeHash to find the most similar block for a given hash.
const FlowBlock *matchWithOpcodes(BlendedBlockHash BlendedHash) const {
@@ -284,7 +294,7 @@ class StaleMatcher {
// Searches for the pseudo probe attached to the matched function's block,
// ignoring pseudo probes attached to function calls and inlined functions'
// blocks.
- outs() << "match with pseudo probes\n";
+ std::vector<const yaml::bolt::PseudoProbeInfo *> BlockPseudoProbes;
for (const auto &PseudoProbe : PseudoProbes) {
// Ensures that pseudo probe information belongs to the appropriate
// function and not an inlined function.
@@ -293,11 +303,30 @@ class StaleMatcher {
// Skips pseudo probes attached to function calls.
if (PseudoProbe.Type != static_cast<uint8_t>(PseudoProbeType::Block))
continue;
- assert(PseudoProbe.Index < Blocks.size() &&
- "pseudo probe index out of range");
- return Blocks[PseudoProbe.Index];
+
+ BlockPseudoProbes.push_back(&PseudoProbe);
}
- return nullptr;
+
+ // Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo
+ // probe and binary pseudo probe.
+ if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1)
+ return nullptr;
+
+ uint64_t Index = BlockPseudoProbes[0]->Index;
+ assert(Index < Blocks.size() && "Invalid pseudo probe index");
+
+ auto It = IndexToBinaryPseudoProbes.find(Index);
+ assert(It != IndexToBinaryPseudoProbes.end() &&
+ "All blocks should have a pseudo probe");
+ if (It->second.size() > 1)
+ return nullptr;
+
+ const MCDecodedPseudoProbe *BinaryPseudoProbe = It->second[0];
+ auto BinaryPseudoProbeIt = BinaryPseudoProbeToBlock.find(BinaryPseudoProbe);
+ assert(BinaryPseudoProbeIt != BinaryPseudoProbeToBlock.end() &&
+ "All binary pseudo probes should belong a binary basic block");
+
+ return BinaryPseudoProbeIt->second;
}
};
@@ -491,6 +520,11 @@ size_t matchWeightsByHashes(
std::vector<uint64_t> CallHashes;
std::vector<FlowBlock *> Blocks;
std::vector<BlendedBlockHash> BlendedHashes;
+ std::unordered_map<uint64_t, std::vector<const MCDecodedPseudoProbe *>>
+ IndexToBinaryPseudoProbes;
+ std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *>
+ BinaryPseudoProbeToBlock;
+ const MCPseudoProbeDecoder *PseudoProbeDecoder = BC.getPseudoProbeDecoder();
for (uint64_t I = 0; I < BlockOrder.size(); I++) {
const BinaryBasicBlock *BB = BlockOrder[I];
assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
@@ -510,9 +544,27 @@ size_t matchWeightsByHashes(
Blocks.push_back(&Func.Blocks[I + 1]);
BlendedBlockHash BlendedHash(BB->getHash());
BlendedHashes.push_back(BlendedHash);
+ if (PseudoProbeDecoder) {
+ const AddressProbesMap &ProbeMap =
+ PseudoProbeDecoder->getAddress2ProbesMap();
+ const uint64_t FuncAddr = BF.getAddress();
+ const std::pair<uint64_t, uint64_t> &BlockRange =
+ BB->getInputAddressRange();
+ const auto &BlockProbes =
+ llvm::make_range(ProbeMap.lower_bound(FuncAddr + BlockRange.first),
+ ProbeMap.lower_bound(FuncAddr + BlockRange.second));
+ for (const auto &[_, Probes] : BlockProbes) {
+ for (const MCDecodedPseudoProbe &Probe : Probes) {
+ IndexToBinaryPseudoProbes[Probe.getIndex()].push_back(&Probe);
+ BinaryPseudoProbeToBlock[&Probe] = Blocks[I];
+ }
+ }
+ }
+
LLVM_DEBUG(dbgs() << "BB with index " << I << " has hash = "
<< Twine::utohexstr(BB->getHash()) << "\n");
}
+
uint64_t BFPseudoProbeDescHash = 0;
if (BF.hasPseudoProbe()) {
const MCPseudoProbeDecoder *PseudoProbeDecoder = BC.getPseudoProbeDecoder();
@@ -521,14 +573,15 @@ size_t matchWeightsByHashes(
BFPseudoProbeDescHash =
PseudoProbeDecoder->getFuncDescForGUID(BF.getGUID())->FuncHash;
}
- bool MatchWithPseudoProbes =
- BFPseudoProbeDescHash && YamlBF.PseudoProbeDescHash
- ? BFPseudoProbeDescHash == YamlBF.PseudoProbeDescHash
- : false;
+ uint64_t YamlBFGUID =
+ BFPseudoProbeDescHash && YamlBF.PseudoProbeDescHash &&
+ BFPseudoProbeDescHash == YamlBF.PseudoProbeDescHash
+ ? static_cast<uint64_t>(YamlBF.GUID)
+ : 0;
+
StaleMatcher Matcher;
- Matcher.init(Blocks, BlendedHashes, CallHashes,
- MatchWithPseudoProbes ? std::make_optional(YamlBF.GUID)
- : std::nullopt);
+ Matcher.init(Blocks, BlendedHashes, CallHashes, IndexToBinaryPseudoProbes,
+ BinaryPseudoProbeToBlock, YamlBFGUID);
// Index in yaml profile => corresponding (matched) block
DenseMap<uint64_t, const FlowBlock *> MatchedBlocks;
diff --git a/bolt/test/X86/match-blocks-with-pseudo-probes.test b/bolt/test/X86/match-blocks-with-pseudo-probes.test
index e0adb6948e206..1d74b92a11c56 100644
--- a/bolt/test/X86/match-blocks-with-pseudo-probes.test
+++ b/bolt/test/X86/match-blocks-with-pseudo-probes.test
@@ -5,7 +5,7 @@
# 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 --funcs=main --profile-ignore-hash=0 2>&1 | FileCheck %s
+# RUN: --print-cfg --funcs=main --profile-ignore-hash=0 --infer-stale-profile 2>&1 | FileCheck %s
# CHECK: BOLT-INFO: matched 0 functions with similar names
@@ -47,23 +47,16 @@ header:
dfs-order: false
hash-func: xxh3
functions:
- - name: main
- fid: 0
- hash: 0x0000000000000001
- exec: 1
- nblocks: 6
+ - name: main
+ fid: 0
+ hash: 0x0000000000000001
+ exec: 1
+ nblocks: 6
+ guid: 0xDB956436E78DD5FA
+ pseudo_probe_desc_hash: 15822663052811949562 #lookup in code in a second
blocks:
- bid: 1
hash: 0x0000000000000001
insns: 1
succ: [ { bid: 3, cnt: 1} ]
- - name: foo
- fid: 1
- hash: 0x0000000000000002
- exec: 1
- nblocks: 6
- blocks:
- - bid: 1
- hash: 0x0000000000000002
- insns: 1
- succ: [ { bid: 3, cnt: 1} ]
+ pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 0, type: 0 } ]
>From 7e3d8d6b171954836c858f0814befc54f70bd3aa Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 22 Jul 2024 14:27:44 -0700
Subject: [PATCH 2/3] Edit test and assert
Created using spr 1.3.4
---
bolt/lib/Profile/StaleProfileMatching.cpp | 2 +-
bolt/test/X86/match-blocks-with-pseudo-probes.test | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index c135ee5ff4837..71e0579415fc6 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -313,7 +313,7 @@ class StaleMatcher {
return nullptr;
uint64_t Index = BlockPseudoProbes[0]->Index;
- assert(Index < Blocks.size() && "Invalid pseudo probe index");
+ assert(Index <= Blocks.size() && "Invalid pseudo probe index");
auto It = IndexToBinaryPseudoProbes.find(Index);
assert(It != IndexToBinaryPseudoProbes.end() &&
diff --git a/bolt/test/X86/match-blocks-with-pseudo-probes.test b/bolt/test/X86/match-blocks-with-pseudo-probes.test
index 1d74b92a11c56..6dc01eb492eae 100644
--- a/bolt/test/X86/match-blocks-with-pseudo-probes.test
+++ b/bolt/test/X86/match-blocks-with-pseudo-probes.test
@@ -53,10 +53,10 @@ functions:
exec: 1
nblocks: 6
guid: 0xDB956436E78DD5FA
- pseudo_probe_desc_hash: 15822663052811949562 #lookup in code in a second
+ pseudo_probe_desc_hash: 4294967295 #lookup in code in a second
blocks:
- bid: 1
hash: 0x0000000000000001
insns: 1
succ: [ { bid: 3, cnt: 1} ]
- pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 0, type: 0 } ]
+ pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 1, type: 0 } ]
>From 780a07ee5a4b2bc3f5bd6e33fb072d67d1113c89 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 23 Jul 2024 11:37:14 -0700
Subject: [PATCH 3/3] Fixed failing asserts, pruned prospective pseudo probes
for matching
Created using spr 1.3.4
---
bolt/lib/Profile/StaleProfileMatching.cpp | 56 ++++++++++++++++-------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 71e0579415fc6..d45066ed66ef2 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -45,6 +45,7 @@ namespace opts {
extern cl::opt<bool> TimeRewrite;
extern cl::OptionCategory BoltOptCategory;
+extern cl::opt<unsigned> Verbosity;
cl::opt<bool>
InferStaleProfile("infer-stale-profile",
@@ -197,9 +198,9 @@ class StaleMatcher {
const std::vector<uint64_t> &CallHashes,
const std::unordered_map<uint64_t,
std::vector<const MCDecodedPseudoProbe *>>
- IndexToBinaryPseudoProbes,
+ &IndexToBinaryPseudoProbes,
const std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *>
- BinaryPseudoProbeToBlock,
+ &BinaryPseudoProbeToBlock,
const uint64_t YamlBFGUID) {
assert(Blocks.size() == Hashes.size() &&
Hashes.size() == CallHashes.size() &&
@@ -294,6 +295,9 @@ class StaleMatcher {
// Searches for the pseudo probe attached to the matched function's block,
// ignoring pseudo probes attached to function calls and inlined functions'
// blocks.
+ if (opts::Verbosity >= 2)
+ outs() << "BOLT-INFO: attempting to match block with pseudo probes\n";
+
std::vector<const yaml::bolt::PseudoProbeInfo *> BlockPseudoProbes;
for (const auto &PseudoProbe : PseudoProbes) {
// Ensures that pseudo probe information belongs to the appropriate
@@ -306,26 +310,41 @@ class StaleMatcher {
BlockPseudoProbes.push_back(&PseudoProbe);
}
-
// Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo
// probe and binary pseudo probe.
- if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1)
+ if (BlockPseudoProbes.size() == 0) {
+ if (opts::Verbosity >= 2)
+ errs() << "BOLT-WARNING: no pseudo probes in profile block\n";
return nullptr;
-
+ }
+ if (BlockPseudoProbes.size() > 1) {
+ if (opts::Verbosity >= 2)
+ errs() << "BOLT-WARNING: more than 1 pseudo probes in profile block\n";
+ return nullptr;
+ }
uint64_t Index = BlockPseudoProbes[0]->Index;
- assert(Index <= Blocks.size() && "Invalid pseudo probe index");
-
+ if (Index > Blocks.size()) {
+ if (opts::Verbosity >= 2)
+ errs() << "BOLT-WARNING: invalid index block pseudo probe index\n";
+ return nullptr;
+ }
auto It = IndexToBinaryPseudoProbes.find(Index);
- assert(It != IndexToBinaryPseudoProbes.end() &&
- "All blocks should have a pseudo probe");
- if (It->second.size() > 1)
+ if (It == IndexToBinaryPseudoProbes.end()) {
+ if (opts::Verbosity >= 2)
+ errs() << "BOLT-WARNING: no block pseudo probes found within binary "
+ "block at index\n";
return nullptr;
-
+ }
+ if (It->second.size() > 1) {
+ if (opts::Verbosity >= 2)
+ errs() << "BOLT-WARNING: more than 1 block pseudo probes in binary "
+ "block at index\n";
+ return nullptr;
+ }
const MCDecodedPseudoProbe *BinaryPseudoProbe = It->second[0];
auto BinaryPseudoProbeIt = BinaryPseudoProbeToBlock.find(BinaryPseudoProbe);
assert(BinaryPseudoProbeIt != BinaryPseudoProbeToBlock.end() &&
"All binary pseudo probes should belong a binary basic block");
-
return BinaryPseudoProbeIt->second;
}
};
@@ -555,6 +574,10 @@ size_t matchWeightsByHashes(
ProbeMap.lower_bound(FuncAddr + BlockRange.second));
for (const auto &[_, Probes] : BlockProbes) {
for (const MCDecodedPseudoProbe &Probe : Probes) {
+ if (Probe.getInlineTreeNode()->hasInlineSite())
+ continue;
+ if (Probe.getType() != static_cast<uint8_t>(PseudoProbeType::Block))
+ continue;
IndexToBinaryPseudoProbes[Probe.getIndex()].push_back(&Probe);
BinaryPseudoProbeToBlock[&Probe] = Blocks[I];
}
@@ -566,12 +589,13 @@ size_t matchWeightsByHashes(
}
uint64_t BFPseudoProbeDescHash = 0;
- if (BF.hasPseudoProbe()) {
- const MCPseudoProbeDecoder *PseudoProbeDecoder = BC.getPseudoProbeDecoder();
+ if (BF.getGUID() != 0) {
assert(PseudoProbeDecoder &&
"If BF has pseudo probe, BC should have a pseudo probe decoder");
- BFPseudoProbeDescHash =
- PseudoProbeDecoder->getFuncDescForGUID(BF.getGUID())->FuncHash;
+ auto &GUID2FuncDescMap = PseudoProbeDecoder->getGUID2FuncDescMap();
+ auto It = GUID2FuncDescMap.find(BF.getGUID());
+ if (It != GUID2FuncDescMap.end())
+ BFPseudoProbeDescHash = It->second.FuncHash;
}
uint64_t YamlBFGUID =
BFPseudoProbeDescHash && YamlBF.PseudoProbeDescHash &&
More information about the llvm-branch-commits
mailing list