[llvm-branch-commits] [llvm] [BOLT] Match functions with calls as anchors (PR #96596)
Shaw Young via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jul 1 11:42:57 PDT 2024
https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/96596
>From 05d59574d6260b98a469921eb2fccf5398bfafb6 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 24 Jun 2024 23:00:59 -0700
Subject: [PATCH 1/6] Added call to matchWithCallsAsAnchors
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index aafffac3d4b1c..1a0e5d239d252 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -479,6 +479,9 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
if (!YamlBF.Used && BF && !ProfiledFunctions.count(BF))
matchProfileToFunction(YamlBF, *BF);
+ uint64_t MatchedWithCallsAsAnchors = 0;
+ matchWithCallsAsAnchors(BC, MatchedWithCallsAsAnchors);
+
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
if (!YamlBF.Used && opts::Verbosity >= 1)
errs() << "BOLT-WARNING: profile ignored for function " << YamlBF.Name
>From 77ef0008f4f5987719555e6cc3e32da812ae0f31 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 24 Jun 2024 23:11:43 -0700
Subject: [PATCH 2/6] Changed CallHashToBF representation
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 1a0e5d239d252..91b01a99c7485 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -29,6 +29,10 @@ static llvm::cl::opt<bool>
cl::desc("ignore hash while reading function profile"),
cl::Hidden, cl::cat(BoltOptCategory));
+llvm::cl::opt<bool> MatchWithCallsAsAnchors("match-with-calls-as-anchors",
+ cl::desc("Matches with calls as anchors"),
+ 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));
@@ -353,7 +357,7 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
llvm_unreachable("Unhandled HashFunction");
};
- std::unordered_map<uint64_t, BinaryFunction &> CallHashToBF;
+ std::unordered_map<uint64_t, BinaryFunction *> CallHashToBF;
for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
if (ProfiledFunctions.count(BF))
@@ -375,12 +379,12 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
for (const std::string &FunctionName : FunctionNames)
HashString.append(FunctionName);
}
- CallHashToBF.emplace(ComputeCallHash(HashString), BF);
+ CallHashToBF[ComputeCallHash(HashString)] = BF;
}
std::unordered_map<uint32_t, std::string> ProfiledFunctionIdToName;
- for (const yaml::bolt::BinaryFunctionProfile YamlBF : YamlBP.Functions)
+ for (const yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
ProfiledFunctionIdToName[YamlBF.Id] = YamlBF.Name;
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
@@ -401,7 +405,7 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
auto It = CallHashToBF.find(Hash);
if (It == CallHashToBF.end())
continue;
- matchProfileToFunction(YamlBF, It->second);
+ matchProfileToFunction(YamlBF, *It->second);
++MatchedWithCallsAsAnchors;
}
}
@@ -480,7 +484,8 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
matchProfileToFunction(YamlBF, *BF);
uint64_t MatchedWithCallsAsAnchors = 0;
- matchWithCallsAsAnchors(BC, MatchedWithCallsAsAnchors);
+ if (opts::MatchWithCallsAsAnchors)
+ matchWithCallsAsAnchors(BC, MatchedWithCallsAsAnchors);
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
if (!YamlBF.Used && opts::Verbosity >= 1)
>From ea7cb68ab9e8e158412c2e752986968968a60d93 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 25 Jun 2024 09:28:39 -0700
Subject: [PATCH 3/6] Changed BF called FunctionNames to multiset
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 91b01a99c7485..3b3d73f7af023 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -365,7 +365,7 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
std::string HashString;
for (const auto &BB : BF->blocks()) {
- std::set<std::string> FunctionNames;
+ std::multiset<std::string> FunctionNames;
for (const MCInst &Instr : BB) {
// Skip non-call instructions.
if (!BC.MIB->isCall(Instr))
@@ -397,9 +397,8 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
std::string &FunctionName = ProfiledFunctionIdToName[CallSite.DestId];
FunctionNames.insert(FunctionName);
}
- for (const std::string &FunctionName : FunctionNames) {
+ for (const std::string &FunctionName : FunctionNames)
HashString.append(FunctionName);
- }
}
size_t Hash = ComputeCallHash(HashString);
auto It = CallHashToBF.find(Hash);
>From c435034970c831c75ab718a9cb82c77d3004e091 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 25 Jun 2024 11:19:47 -0700
Subject: [PATCH 4/6] GetOneName for YamlBF
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileReader.cpp | 38 +++++++++++++++++++-------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 3b3d73f7af023..2ea1a051d314c 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -29,9 +29,10 @@ static llvm::cl::opt<bool>
cl::desc("ignore hash while reading function profile"),
cl::Hidden, cl::cat(BoltOptCategory));
-llvm::cl::opt<bool> MatchWithCallsAsAnchors("match-with-calls-as-anchors",
- cl::desc("Matches with calls as anchors"),
- cl::Hidden, cl::cat(BoltOptCategory));
+llvm::cl::opt<bool>
+ MatchWithCallsAsAnchors("match-with-calls-as-anchors",
+ cl::desc("Matches with calls as anchors"),
+ cl::Hidden, cl::cat(BoltOptCategory));
llvm::cl::opt<bool> ProfileUseDFS("profile-use-dfs",
cl::desc("use DFS order for YAML profile"),
@@ -358,12 +359,11 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
};
std::unordered_map<uint64_t, BinaryFunction *> CallHashToBF;
-
for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
if (ProfiledFunctions.count(BF))
continue;
- std::string HashString;
+ std::string HashString{""};
for (const auto &BB : BF->blocks()) {
std::multiset<std::string> FunctionNames;
for (const MCInst &Instr : BB) {
@@ -379,14 +379,23 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
for (const std::string &FunctionName : FunctionNames)
HashString.append(FunctionName);
}
+ // its possible we have some collisions.. our options to solve include
+ // cmp block ct, or potentially fname edit distance. although, thats p exp
+ if (HashString == "")
+ continue;
CallHashToBF[ComputeCallHash(HashString)] = BF;
}
std::unordered_map<uint32_t, std::string> ProfiledFunctionIdToName;
-
- for (const yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
- ProfiledFunctionIdToName[YamlBF.Id] = YamlBF.Name;
-
+ for (const yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
+ // How do we handle functions with multiple names? LTO specifically,
+ // right now the scope of this is to identical names wo lto i think
+ StringRef Name = YamlBF.Name;
+ const size_t Pos = Name.find("(*");
+ if (Pos != StringRef::npos)
+ Name = Name.substr(0, Pos);
+ ProfiledFunctionIdToName[YamlBF.Id] = Name;
+ }
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
if (YamlBF.Used)
continue;
@@ -400,10 +409,14 @@ void YAMLProfileReader::matchWithCallsAsAnchors(
for (const std::string &FunctionName : FunctionNames)
HashString.append(FunctionName);
}
+ if (HashString == "")
+ continue;
size_t Hash = ComputeCallHash(HashString);
auto It = CallHashToBF.find(Hash);
if (It == CallHashToBF.end())
continue;
+ if (ProfiledFunctions.count(It->second))
+ continue;
matchProfileToFunction(YamlBF, *It->second);
++MatchedWithCallsAsAnchors;
}
@@ -484,13 +497,18 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
uint64_t MatchedWithCallsAsAnchors = 0;
if (opts::MatchWithCallsAsAnchors)
- matchWithCallsAsAnchors(BC, MatchedWithCallsAsAnchors);
+ matchWithCallsAsAnchors(BC, MatchedWithCallsAsAnchors);
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
if (!YamlBF.Used && opts::Verbosity >= 1)
errs() << "BOLT-WARNING: profile ignored for function " << YamlBF.Name
<< '\n';
+ if (opts::Verbosity >= 2) {
+ outs() << "BOLT-INFO: matched " << MatchedWithCallsAsAnchors
+ << " functions with calls as anchors\n";
+ }
+
// Set for parseFunctionProfile().
NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions");
NormalizeByCalls = usesEvent("branches");
>From 42363169b83a0ca7101187670b20d0e2167f2fbe Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Thu, 27 Jun 2024 16:20:47 -0700
Subject: [PATCH 5/6] Moved to StaleProfileMatching
Created using spr 1.3.4
---
bolt/include/bolt/Core/HashUtilities.h | 2 +
bolt/include/bolt/Profile/YAMLProfileReader.h | 4 -
bolt/lib/Core/HashUtilities.cpp | 25 ++++++
bolt/lib/Profile/StaleProfileMatching.cpp | 51 ++++++++---
bolt/lib/Profile/YAMLProfileReader.cpp | 89 -------------------
5 files changed, 65 insertions(+), 106 deletions(-)
diff --git a/bolt/include/bolt/Core/HashUtilities.h b/bolt/include/bolt/Core/HashUtilities.h
index 53ea110aa683b..c86cd906b3f34 100644
--- a/bolt/include/bolt/Core/HashUtilities.h
+++ b/bolt/include/bolt/Core/HashUtilities.h
@@ -35,6 +35,8 @@ std::string hashBlock(BinaryContext &BC, const BinaryBasicBlock &BB,
std::string hashBlockLoose(BinaryContext &BC, const BinaryBasicBlock &BB);
+std::string hashBlockCalls(BinaryContext &BC, const BinaryBasicBlock &BB);
+
} // namespace bolt
} // namespace llvm
diff --git a/bolt/include/bolt/Profile/YAMLProfileReader.h b/bolt/include/bolt/Profile/YAMLProfileReader.h
index 572e8a66c5b50..7a8aa176c30f1 100644
--- a/bolt/include/bolt/Profile/YAMLProfileReader.h
+++ b/bolt/include/bolt/Profile/YAMLProfileReader.h
@@ -73,10 +73,6 @@ class YAMLProfileReader : public ProfileReaderBase {
bool parseFunctionProfile(BinaryFunction &Function,
const yaml::bolt::BinaryFunctionProfile &YamlBF);
- /// Match profiles to functions using calls as anchors.
- void matchWithCallsAsAnchors(BinaryContext &BC,
- uint64_t &MatchedWithCallsAsAnchors);
-
/// Infer function profile from stale data (collected on older binaries).
bool inferStaleProfile(BinaryFunction &Function,
const yaml::bolt::BinaryFunctionProfile &YamlBF);
diff --git a/bolt/lib/Core/HashUtilities.cpp b/bolt/lib/Core/HashUtilities.cpp
index c4c67bd68198b..7d72dbe8928ab 100644
--- a/bolt/lib/Core/HashUtilities.cpp
+++ b/bolt/lib/Core/HashUtilities.cpp
@@ -155,5 +155,30 @@ std::string hashBlockLoose(BinaryContext &BC, const BinaryBasicBlock &BB) {
return HashString;
}
+/// An even looser hash of a basic block to use with the stale profile
+/// matching. Hashing block function calls allows for broader relational
+/// matching, predicated on the assumption that called functions remain similar
+/// across revisions.
+std::string hashBlockCalls(BinaryContext &BC, const BinaryBasicBlock &BB) {
+ // The hash is computed by creating a string of all lexicographically ordered
+ // called function names.
+ std::multiset<std::string> FunctionNames;
+ for (const MCInst &Instr : BB) {
+ // Skip non-call instructions.
+ if (!BC.MIB->isCall(Instr))
+ continue;
+ const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
+ if (!CallSymbol)
+ continue;
+ FunctionNames.insert(std::string(CallSymbol->getName()));
+ }
+
+ std::string HashString;
+ for (const std::string &FunctionName : FunctionNames)
+ HashString.append(FunctionName);
+
+ return HashString;
+}
+
} // namespace bolt
} // namespace llvm
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 365bc5389266d..5b318e6793069 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -152,8 +152,8 @@ struct BlendedBlockHash {
/// distance, the more similar two blocks are. For identical basic blocks,
/// the distance is zero.
uint64_t distance(const BlendedBlockHash &BBH) const {
- assert(OpcodeHash == BBH.OpcodeHash &&
- "incorrect blended hash distance computation");
+ // assert(OpcodeHash == BBH.OpcodeHash &&
+ // "incorrect blended hash distance computation");
uint64_t Dist = 0;
// Account for NeighborHash
Dist += SuccHash == BBH.SuccHash ? 0 : 1;
@@ -187,20 +187,22 @@ class StaleMatcher {
public:
/// Initialize stale matcher.
void init(const std::vector<FlowBlock *> &Blocks,
- const std::vector<BlendedBlockHash> &Hashes) {
+ const std::vector<BlendedBlockHash> &Hashes,
+ const std::vector<uint64_t> &CallHashes) {
assert(Blocks.size() == Hashes.size() &&
"incorrect matcher initialization");
for (size_t I = 0; I < Blocks.size(); I++) {
FlowBlock *Block = Blocks[I];
- uint16_t OpHash = Hashes[I].OpcodeHash;
- OpHashToBlocks[OpHash].push_back(std::make_pair(Hashes[I], Block));
+ uint16_t CallHash = CallHashes[I];
+ CallHashToBlocks[CallHash].push_back(std::make_pair(Hashes[I], Block));
}
}
/// Find the most similar block for a given hash.
- const FlowBlock *matchBlock(BlendedBlockHash BlendedHash) const {
- auto BlockIt = OpHashToBlocks.find(BlendedHash.OpcodeHash);
- if (BlockIt == OpHashToBlocks.end())
+ const FlowBlock *matchBlock(uint64_t CallHash,
+ BlendedBlockHash BlendedHash) const {
+ auto BlockIt = CallHashToBlocks.find(CallHash);
+ if (BlockIt == CallHashToBlocks.end())
return nullptr;
FlowBlock *BestBlock = nullptr;
uint64_t BestDist = std::numeric_limits<uint64_t>::max();
@@ -222,9 +224,11 @@ class StaleMatcher {
return Hash1.InstrHash == Hash2.InstrHash;
}
+ // TODO: add "mid confidence match?" for loose hash matching
+
private:
using HashBlockPairType = std::pair<BlendedBlockHash, FlowBlock *>;
- std::unordered_map<uint16_t, std::vector<HashBlockPairType>> OpHashToBlocks;
+ std::unordered_map<uint64_t, std::vector<HashBlockPairType>> CallHashToBlocks;
};
void BinaryFunction::computeBlockHashes(HashFunction HashFunction) const {
@@ -391,17 +395,38 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
/// of the basic blocks in the binary, the count is "matched" to the block.
/// Similarly, if both the source and the target of a count in the profile are
/// matched to a jump in the binary, the count is recorded in CFG.
-void matchWeightsByHashes(BinaryContext &BC,
+void matchWeightsByHashes(HashFunction HashFunction,
+ BinaryContext &BC,
const BinaryFunction::BasicBlockOrderType &BlockOrder,
const yaml::bolt::BinaryFunctionProfile &YamlBF,
FlowFunction &Func) {
+ // TODO: perhaps in the interest of polish we move blended hashes, callhashes,
+ // and blocks into an "intialize stale matcher" function and use it here
+ // or in inferStaleProfile
+
assert(Func.Blocks.size() == BlockOrder.size() + 1);
+ std::vector<uint64_t> CallHashes;
std::vector<FlowBlock *> Blocks;
std::vector<BlendedBlockHash> BlendedHashes;
+
+ CallHashes.resize(BlockOrder.size());
+ Blocks.resize(BlockOrder.size());
+ BlendedHashes.resize(BlockOrder.size());
+
for (uint64_t I = 0; I < BlockOrder.size(); I++) {
const BinaryBasicBlock *BB = BlockOrder[I];
assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
+
+ std::string CallHashStr = hashBlockCalls(BC, *BB);
+ if (HashFunction == HashFunction::StdHash) {
+ CallHashes.push_back(std::hash<std::string>{}(CallHashStr));
+ } else if (HashFunction == HashFunction::XXH3) {
+ CallHashes.push_back(llvm::xxh3_64bits(CallHashStr));
+ } else {
+ llvm_unreachable("Unhandled HashFunction");
+ }
+
Blocks.push_back(&Func.Blocks[I + 1]);
BlendedBlockHash BlendedHash(BB->getHash());
BlendedHashes.push_back(BlendedHash);
@@ -409,7 +434,7 @@ void matchWeightsByHashes(BinaryContext &BC,
<< Twine::utohexstr(BB->getHash()) << "\n");
}
StaleMatcher Matcher;
- Matcher.init(Blocks, BlendedHashes);
+ Matcher.init(Blocks, BlendedHashes, CallHashes);
// Index in yaml profile => corresponding (matched) block
DenseMap<uint64_t, const FlowBlock *> MatchedBlocks;
@@ -417,7 +442,7 @@ void matchWeightsByHashes(BinaryContext &BC,
for (const yaml::bolt::BinaryBasicBlockProfile &YamlBB : YamlBF.Blocks) {
assert(YamlBB.Hash != 0 && "empty hash of BinaryBasicBlockProfile");
BlendedBlockHash YamlHash(YamlBB.Hash);
- const FlowBlock *MatchedBlock = Matcher.matchBlock(YamlHash);
+ const FlowBlock *MatchedBlock = Matcher.matchBlock(CallHashes[YamlBB.Index], YamlHash);
// Always match the entry block.
if (MatchedBlock == nullptr && YamlBB.Index == 0)
MatchedBlock = Blocks[0];
@@ -729,7 +754,7 @@ bool YAMLProfileReader::inferStaleProfile(
FlowFunction Func = createFlowFunction(BlockOrder);
// Match as many block/jump counts from the stale profile as possible
- matchWeightsByHashes(BF.getBinaryContext(), BlockOrder, YamlBF, Func);
+ matchWeightsByHashes(YamlBP.Header.HashFunction, BF.getBinaryContext(), BlockOrder, YamlBF, Func);
// Adjust the flow function by marking unreachable blocks Unlikely so that
// they don't get any counts assigned.
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 2ea1a051d314c..f25f59201f1cd 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -14,7 +14,6 @@
#include "bolt/Utils/Utils.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/xxhash.h"
using namespace llvm;
@@ -29,11 +28,6 @@ static llvm::cl::opt<bool>
cl::desc("ignore hash while reading function profile"),
cl::Hidden, cl::cat(BoltOptCategory));
-llvm::cl::opt<bool>
- MatchWithCallsAsAnchors("match-with-calls-as-anchors",
- cl::desc("Matches with calls as anchors"),
- 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));
@@ -348,80 +342,6 @@ bool YAMLProfileReader::mayHaveProfileData(const BinaryFunction &BF) {
return false;
}
-void YAMLProfileReader::matchWithCallsAsAnchors(
- BinaryContext &BC, uint64_t &MatchedWithCallsAsAnchors) {
- auto ComputeCallHash = [&](std::string HashString) {
- if (YamlBP.Header.HashFunction == HashFunction::StdHash)
- return std::hash<std::string>{}(HashString);
- if (YamlBP.Header.HashFunction == HashFunction::XXH3)
- return llvm::xxh3_64bits(HashString);
- llvm_unreachable("Unhandled HashFunction");
- };
-
- std::unordered_map<uint64_t, BinaryFunction *> CallHashToBF;
- for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
- if (ProfiledFunctions.count(BF))
- continue;
-
- std::string HashString{""};
- for (const auto &BB : BF->blocks()) {
- std::multiset<std::string> FunctionNames;
- for (const MCInst &Instr : BB) {
- // Skip non-call instructions.
- if (!BC.MIB->isCall(Instr))
- continue;
- const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
- if (!CallSymbol)
- continue;
- FunctionNames.insert(std::string(CallSymbol->getName()));
- }
- // Adds called functions to the hash string in lexigraphic order.
- for (const std::string &FunctionName : FunctionNames)
- HashString.append(FunctionName);
- }
- // its possible we have some collisions.. our options to solve include
- // cmp block ct, or potentially fname edit distance. although, thats p exp
- if (HashString == "")
- continue;
- CallHashToBF[ComputeCallHash(HashString)] = BF;
- }
-
- std::unordered_map<uint32_t, std::string> ProfiledFunctionIdToName;
- for (const yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
- // How do we handle functions with multiple names? LTO specifically,
- // right now the scope of this is to identical names wo lto i think
- StringRef Name = YamlBF.Name;
- const size_t Pos = Name.find("(*");
- if (Pos != StringRef::npos)
- Name = Name.substr(0, Pos);
- ProfiledFunctionIdToName[YamlBF.Id] = Name;
- }
- for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
- if (YamlBF.Used)
- continue;
- std::string HashString{""};
- for (const yaml::bolt::BinaryBasicBlockProfile &Block : YamlBF.Blocks) {
- std::multiset<std::string> FunctionNames;
- for (const yaml::bolt::CallSiteInfo &CallSite : Block.CallSites) {
- std::string &FunctionName = ProfiledFunctionIdToName[CallSite.DestId];
- FunctionNames.insert(FunctionName);
- }
- for (const std::string &FunctionName : FunctionNames)
- HashString.append(FunctionName);
- }
- if (HashString == "")
- continue;
- size_t Hash = ComputeCallHash(HashString);
- auto It = CallHashToBF.find(Hash);
- if (It == CallHashToBF.end())
- continue;
- if (ProfiledFunctions.count(It->second))
- continue;
- matchProfileToFunction(YamlBF, *It->second);
- ++MatchedWithCallsAsAnchors;
- }
-}
-
Error YAMLProfileReader::readProfile(BinaryContext &BC) {
if (opts::Verbosity >= 1) {
outs() << "BOLT-INFO: YAML profile with hash: ";
@@ -495,20 +415,11 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
if (!YamlBF.Used && BF && !ProfiledFunctions.count(BF))
matchProfileToFunction(YamlBF, *BF);
- uint64_t MatchedWithCallsAsAnchors = 0;
- if (opts::MatchWithCallsAsAnchors)
- matchWithCallsAsAnchors(BC, MatchedWithCallsAsAnchors);
-
for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
if (!YamlBF.Used && opts::Verbosity >= 1)
errs() << "BOLT-WARNING: profile ignored for function " << YamlBF.Name
<< '\n';
- if (opts::Verbosity >= 2) {
- outs() << "BOLT-INFO: matched " << MatchedWithCallsAsAnchors
- << " functions with calls as anchors\n";
- }
-
// Set for parseFunctionProfile().
NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions");
NormalizeByCalls = usesEvent("branches");
>From 5f83647ae12a2165e18415cb213bec341fc319f2 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 1 Jul 2024 11:42:46 -0700
Subject: [PATCH 6/6] Added CallHash check following OpHash check in block
matchgin
Created using spr 1.3.4
---
bolt/include/bolt/Core/HashUtilities.h | 6 +
.../include/bolt/Profile/ProfileYAMLMapping.h | 1 +
bolt/lib/Core/HashUtilities.cpp | 26 ++++
bolt/lib/Profile/StaleProfileMatching.cpp | 112 ++++++++++++------
bolt/lib/Profile/YAMLProfileReader.cpp | 4 +
5 files changed, 113 insertions(+), 36 deletions(-)
diff --git a/bolt/include/bolt/Core/HashUtilities.h b/bolt/include/bolt/Core/HashUtilities.h
index c86cd906b3f34..c84bbad0ba29b 100644
--- a/bolt/include/bolt/Core/HashUtilities.h
+++ b/bolt/include/bolt/Core/HashUtilities.h
@@ -16,6 +16,7 @@
#include "bolt/Core/BinaryBasicBlock.h"
#include "bolt/Core/BinaryContext.h"
+#include "bolt/Profile/ProfileYAMLMapping.h"
namespace llvm {
namespace bolt {
@@ -37,6 +38,11 @@ std::string hashBlockLoose(BinaryContext &BC, const BinaryBasicBlock &BB);
std::string hashBlockCalls(BinaryContext &BC, const BinaryBasicBlock &BB);
+std::string hashBlockCalls(
+ const std::unordered_map<uint32_t, yaml::bolt::BinaryFunctionProfile *>
+ &IdsToProfiledFunctions,
+ const yaml::bolt::BinaryBasicBlockProfile &YamlBB);
+
} // namespace bolt
} // namespace llvm
diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
index 9dd3920dbf094..35a195700a0d5 100644
--- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
+++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
@@ -225,6 +225,7 @@ namespace bolt {
struct BinaryProfile {
BinaryProfileHeader Header;
std::vector<BinaryFunctionProfile> Functions;
+ std::unordered_map<uint32_t, BinaryFunctionProfile *> IdToFunctionProfile;
};
} // namespace bolt
diff --git a/bolt/lib/Core/HashUtilities.cpp b/bolt/lib/Core/HashUtilities.cpp
index 7d72dbe8928ab..601c597126ec7 100644
--- a/bolt/lib/Core/HashUtilities.cpp
+++ b/bolt/lib/Core/HashUtilities.cpp
@@ -180,5 +180,31 @@ std::string hashBlockCalls(BinaryContext &BC, const BinaryBasicBlock &BB) {
return HashString;
}
+/// The same as the above function, but for profiled functions.
+std::string hashBlockCalls(
+ const std::unordered_map<uint32_t, yaml::bolt::BinaryFunctionProfile *>
+ &IdsToProfiledFunctions,
+ const yaml::bolt::BinaryBasicBlockProfile &YamlBB) {
+
+ std::multiset<std::string> FunctionNames;
+ for (const yaml::bolt::CallSiteInfo &CallSiteInfo : YamlBB.CallSites) {
+ auto It = IdsToProfiledFunctions.find(CallSiteInfo.DestId);
+ assert(It != IdsToProfiledFunctions.end() &&
+ "All profiled functions should have ids");
+ StringRef Name = It->second->Name;
+ const size_t Pos = Name.find("(*");
+ if (Pos != StringRef::npos)
+ Name = Name.substr(0, Pos);
+ FunctionNames.insert(std::string(Name));
+ }
+
+ std::string HashString;
+ for (const std::string &FunctionName : FunctionNames)
+ HashString.append(FunctionName);
+
+ return HashString;
+}
+//
+
} // namespace bolt
} // namespace llvm
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 5b318e6793069..cec7ced707ba0 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -152,8 +152,8 @@ struct BlendedBlockHash {
/// distance, the more similar two blocks are. For identical basic blocks,
/// the distance is zero.
uint64_t distance(const BlendedBlockHash &BBH) const {
- // assert(OpcodeHash == BBH.OpcodeHash &&
- // "incorrect blended hash distance computation");
+ assert(OpcodeHash == BBH.OpcodeHash &&
+ "incorrect blended hash distance computation");
uint64_t Dist = 0;
// Account for NeighborHash
Dist += SuccHash == BBH.SuccHash ? 0 : 1;
@@ -188,21 +188,43 @@ class StaleMatcher {
/// Initialize stale matcher.
void init(const std::vector<FlowBlock *> &Blocks,
const std::vector<BlendedBlockHash> &Hashes,
- const std::vector<uint64_t> &CallHashes) {
+ const std::vector<std::unique_ptr<uint64_t>> &CallHashes) {
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 CallHash = CallHashes[I];
- CallHashToBlocks[CallHash].push_back(std::make_pair(Hashes[I], Block));
+ uint16_t OpHash = Hashes[I].OpcodeHash;
+ OpHashToBlocks[OpHash].push_back(std::make_pair(Hashes[I], Block));
+ if (CallHashes[I].get())
+ CallHashToBlocks[*CallHashes[I]].push_back(
+ std::make_pair(Hashes[I], Block));
}
}
/// Find the most similar block for a given hash.
- const FlowBlock *matchBlock(uint64_t CallHash,
- BlendedBlockHash BlendedHash) const {
- auto BlockIt = CallHashToBlocks.find(CallHash);
- if (BlockIt == CallHashToBlocks.end())
+ const FlowBlock *matchBlock(BlendedBlockHash &BlendedHash,
+ std::unique_ptr<uint64_t> &CallHash) const {
+ const FlowBlock *BestBlock = matchWithOpcodes(BlendedHash);
+ return BestBlock ? BestBlock : matchWithCalls(BlendedHash, CallHash);
+ }
+
+ /// Returns true if the two basic blocks (in the binary and in the profile)
+ /// corresponding to the given hashes are matched to each other with a high
+ /// confidence.
+ static bool isHighConfidenceMatch(BlendedBlockHash Hash1,
+ BlendedBlockHash Hash2) {
+ return Hash1.InstrHash == Hash2.InstrHash;
+ }
+
+private:
+ using HashBlockPairType = std::pair<BlendedBlockHash, FlowBlock *>;
+ std::unordered_map<uint16_t, std::vector<HashBlockPairType>> OpHashToBlocks;
+ std::unordered_map<uint64_t, std::vector<HashBlockPairType>> CallHashToBlocks;
+
+ const FlowBlock *matchWithOpcodes(BlendedBlockHash &BlendedHash) const {
+ auto BlockIt = OpHashToBlocks.find(BlendedHash.OpcodeHash);
+ if (BlockIt == OpHashToBlocks.end())
return nullptr;
FlowBlock *BestBlock = nullptr;
uint64_t BestDist = std::numeric_limits<uint64_t>::max();
@@ -216,19 +238,24 @@ class StaleMatcher {
return BestBlock;
}
- /// Returns true if the two basic blocks (in the binary and in the profile)
- /// corresponding to the given hashes are matched to each other with a high
- /// confidence.
- static bool isHighConfidenceMatch(BlendedBlockHash Hash1,
- BlendedBlockHash Hash2) {
- return Hash1.InstrHash == Hash2.InstrHash;
+ const FlowBlock *matchWithCalls(BlendedBlockHash &BlendedHash,
+ std::unique_ptr<uint64_t> &CallHash) const {
+ if (!CallHash.get())
+ return nullptr;
+ auto BlockIt = CallHashToBlocks.find(*CallHash);
+ if (BlockIt == CallHashToBlocks.end())
+ return nullptr;
+ FlowBlock *BestBlock = nullptr;
+ uint64_t BestDist = std::numeric_limits<uint64_t>::max();
+ for (const auto &[Hash, Block] : BlockIt->second) {
+ uint64_t Dist = Hash.distance(BlendedHash);
+ if (BestBlock == nullptr || Dist < BestDist) {
+ BestDist = Dist;
+ BestBlock = Block;
+ }
+ }
+ return BestBlock;
}
-
- // TODO: add "mid confidence match?" for loose hash matching
-
-private:
- using HashBlockPairType = std::pair<BlendedBlockHash, FlowBlock *>;
- std::unordered_map<uint64_t, std::vector<HashBlockPairType>> CallHashToBlocks;
};
void BinaryFunction::computeBlockHashes(HashFunction HashFunction) const {
@@ -395,34 +422,33 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
/// of the basic blocks in the binary, the count is "matched" to the block.
/// Similarly, if both the source and the target of a count in the profile are
/// matched to a jump in the binary, the count is recorded in CFG.
-void matchWeightsByHashes(HashFunction HashFunction,
- BinaryContext &BC,
+void matchWeightsByHashes(BinaryContext &BC,
const BinaryFunction::BasicBlockOrderType &BlockOrder,
+ const yaml::bolt::BinaryProfile &YamlBP,
const yaml::bolt::BinaryFunctionProfile &YamlBF,
FlowFunction &Func) {
- // TODO: perhaps in the interest of polish we move blended hashes, callhashes,
- // and blocks into an "intialize stale matcher" function and use it here
- // or in inferStaleProfile
assert(Func.Blocks.size() == BlockOrder.size() + 1);
- std::vector<uint64_t> CallHashes;
+ std::vector<std::unique_ptr<uint64_t>> CallHashes;
std::vector<FlowBlock *> Blocks;
std::vector<BlendedBlockHash> BlendedHashes;
- CallHashes.resize(BlockOrder.size());
- Blocks.resize(BlockOrder.size());
- BlendedHashes.resize(BlockOrder.size());
+ HashFunction HashFunction = YamlBP.Header.HashFunction;
for (uint64_t I = 0; I < BlockOrder.size(); I++) {
const BinaryBasicBlock *BB = BlockOrder[I];
assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
std::string CallHashStr = hashBlockCalls(BC, *BB);
- if (HashFunction == HashFunction::StdHash) {
- CallHashes.push_back(std::hash<std::string>{}(CallHashStr));
+ if (CallHashStr.empty()) {
+ CallHashes.emplace_back(nullptr);
+ } else if (HashFunction == HashFunction::StdHash) {
+ CallHashes.emplace_back(
+ std::make_unique<uint64_t>(std::hash<std::string>{}(CallHashStr)));
} else if (HashFunction == HashFunction::XXH3) {
- CallHashes.push_back(llvm::xxh3_64bits(CallHashStr));
+ CallHashes.emplace_back(
+ std::make_unique<uint64_t>(llvm::xxh3_64bits(CallHashStr)));
} else {
llvm_unreachable("Unhandled HashFunction");
}
@@ -433,6 +459,7 @@ void matchWeightsByHashes(HashFunction HashFunction,
LLVM_DEBUG(dbgs() << "BB with index " << I << " has hash = "
<< Twine::utohexstr(BB->getHash()) << "\n");
}
+
StaleMatcher Matcher;
Matcher.init(Blocks, BlendedHashes, CallHashes);
@@ -442,8 +469,21 @@ void matchWeightsByHashes(HashFunction HashFunction,
for (const yaml::bolt::BinaryBasicBlockProfile &YamlBB : YamlBF.Blocks) {
assert(YamlBB.Hash != 0 && "empty hash of BinaryBasicBlockProfile");
BlendedBlockHash YamlHash(YamlBB.Hash);
- const FlowBlock *MatchedBlock = Matcher.matchBlock(CallHashes[YamlBB.Index], YamlHash);
- // Always match the entry block.
+
+ const FlowBlock *MatchedBlock = nullptr;
+ std::string CallHashStr =
+ hashBlockCalls(YamlBP.IdToFunctionProfile, YamlBB);
+ std::unique_ptr<uint64_t> CallHash = nullptr;
+ if (HashFunction == HashFunction::StdHash) {
+ CallHash =
+ std::make_unique<uint64_t>(std::hash<std::string>{}(CallHashStr));
+ } else if (HashFunction == HashFunction::XXH3) {
+ CallHash = std::make_unique<uint64_t>(llvm::xxh3_64bits(CallHashStr));
+ } else {
+ llvm_unreachable("Unhandled HashFunction");
+ }
+
+ MatchedBlock = Matcher.matchBlock(YamlHash, CallHash);
if (MatchedBlock == nullptr && YamlBB.Index == 0)
MatchedBlock = Blocks[0];
if (MatchedBlock != nullptr) {
@@ -754,7 +794,7 @@ bool YAMLProfileReader::inferStaleProfile(
FlowFunction Func = createFlowFunction(BlockOrder);
// Match as many block/jump counts from the stale profile as possible
- matchWeightsByHashes(YamlBP.Header.HashFunction, BF.getBinaryContext(), BlockOrder, YamlBF, Func);
+ matchWeightsByHashes(BF.getBinaryContext(), BlockOrder, YamlBP, YamlBF, Func);
// Adjust the flow function by marking unreachable blocks Unlikely so that
// they don't get any counts assigned.
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index f25f59201f1cd..085596daebaed 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -325,6 +325,10 @@ Error YAMLProfileReader::preprocessProfile(BinaryContext &BC) {
}
}
+ // Map profiled function ids to names.
+ for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions)
+ YamlBP.IdToFunctionProfile[YamlBF.Id] = &YamlBF;
+
return Error::success();
}
More information about the llvm-branch-commits
mailing list