[llvm] [BOLT][NFC] Eliminate uses of throwing std::map::at (PR #92950)
shaw young via llvm-commits
llvm-commits at lists.llvm.org
Tue May 21 14:43:39 PDT 2024
https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/92950
>From 37d687a4d4aa94b8e7838cc6184da1ba2a90229f Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 21 May 2024 11:29:23 -0700
Subject: [PATCH 1/3] [BOLT] [NFC] Remove std at calls
---
.../bolt/Profile/BoltAddressTranslation.h | 4 +-
bolt/lib/Core/BinaryContext.cpp | 11 +++--
bolt/lib/Core/BinaryEmitter.cpp | 4 +-
bolt/lib/Core/DynoStats.cpp | 5 ++-
bolt/lib/Passes/BinaryFunctionCallGraph.cpp | 4 +-
bolt/lib/Passes/BinaryPasses.cpp | 22 ++++++++--
bolt/lib/Passes/CacheMetrics.cpp | 40 +++++++++++++++----
bolt/lib/Passes/Inliner.cpp | 4 +-
bolt/lib/Profile/StaleProfileMatching.cpp | 6 ++-
9 files changed, 77 insertions(+), 23 deletions(-)
diff --git a/bolt/include/bolt/Profile/BoltAddressTranslation.h b/bolt/include/bolt/Profile/BoltAddressTranslation.h
index 68b993ee363cc..7005dafe752ec 100644
--- a/bolt/include/bolt/Profile/BoltAddressTranslation.h
+++ b/bolt/include/bolt/Profile/BoltAddressTranslation.h
@@ -278,7 +278,9 @@ class BoltAddressTranslation {
/// Returns the number of basic blocks in a function.
size_t getNumBasicBlocks(uint64_t OutputAddress) const {
- return NumBasicBlocksMap.at(OutputAddress);
+ auto It = NumBasicBlocksMap.find(OutputAddress);
+ assert(It != NumBasicBlocksMap.end());
+ return It->second;
}
private:
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index ad2eb18caf109..046c5a3c0de06 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -934,10 +934,13 @@ std::string BinaryContext::generateJumpTableName(const BinaryFunction &BF,
uint64_t Offset = 0;
if (const JumpTable *JT = BF.getJumpTableContainingAddress(Address)) {
Offset = Address - JT->getAddress();
- auto Itr = JT->Labels.find(Offset);
- if (Itr != JT->Labels.end())
- return std::string(Itr->second->getName());
- Id = JumpTableIds.at(JT->getAddress());
+ auto JTLabelsIt = JT->Labels.find(Offset);
+ if ( JTLabelsIt != JT->Labels.end())
+ return std::string(JTLabelsIt ->second->getName());
+
+ auto JTIdsIt = JumpTableIds.find(JT->getAddress());
+ assert(JTIdsIt != JumpTableIds.end());
+ Id = JTIdsIt->second;
} else {
Id = JumpTableIds[Address] = BF.JumpTables.size();
}
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 6f86ddc774544..0b44acb0816f2 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -813,7 +813,9 @@ void BinaryEmitter::emitJumpTable(const JumpTable &JT, MCSection *HotSection,
// determining its destination.
std::map<MCSymbol *, uint64_t> LabelCounts;
if (opts::JumpTables > JTS_SPLIT && !JT.Counts.empty()) {
- MCSymbol *CurrentLabel = JT.Labels.at(0);
+ auto It = JT.Labels.find(0);
+ assert(It != JT.Labels.end());
+ MCSymbol *CurrentLabel = It->second;
uint64_t CurrentLabelCount = 0;
for (unsigned Index = 0; Index < JT.Entries.size(); ++Index) {
auto LI = JT.Labels.find(Index * JT.EntrySize);
diff --git a/bolt/lib/Core/DynoStats.cpp b/bolt/lib/Core/DynoStats.cpp
index 5de0f9e0d6b8c..1d9818777596e 100644
--- a/bolt/lib/Core/DynoStats.cpp
+++ b/bolt/lib/Core/DynoStats.cpp
@@ -114,8 +114,9 @@ void DynoStats::print(raw_ostream &OS, const DynoStats *Other,
for (auto &Stat : llvm::reverse(SortedHistogram)) {
OS << format("%20s,%'18lld", Printer->getOpcodeName(Stat.second).data(),
Stat.first * opts::DynoStatsScale);
-
- MaxOpcodeHistogramTy MaxMultiMap = OpcodeHistogram.at(Stat.second).second;
+ auto It = OpcodeHistogram.find(Stat.second);
+ assert(It != OpcodeHistogram.end());
+ MaxOpcodeHistogramTy MaxMultiMap = It->second.second;
// Start with function name:BB offset with highest execution count.
for (auto &Max : llvm::reverse(MaxMultiMap)) {
OS << format(", %'18lld, ", Max.first * opts::DynoStatsScale)
diff --git a/bolt/lib/Passes/BinaryFunctionCallGraph.cpp b/bolt/lib/Passes/BinaryFunctionCallGraph.cpp
index 2373710c9edd6..bbcc9751c0cbe 100644
--- a/bolt/lib/Passes/BinaryFunctionCallGraph.cpp
+++ b/bolt/lib/Passes/BinaryFunctionCallGraph.cpp
@@ -56,7 +56,9 @@ std::deque<BinaryFunction *> BinaryFunctionCallGraph::buildTraversalOrder() {
std::stack<NodeId> Worklist;
for (BinaryFunction *Func : Funcs) {
- const NodeId Id = FuncToNodeId.at(Func);
+ auto It = FuncToNodeId.find(Func);
+ assert(It != FuncToNodeId.end());
+ const NodeId Id = It->second;
Worklist.push(Id);
NodeStatus[Id] = NEW;
}
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index 867f977cebca7..713a909afe1a6 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -1553,15 +1553,29 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
llvm::stable_sort(Functions,
[Ascending, &Stats](const BinaryFunction *A,
const BinaryFunction *B) {
- return Ascending ? Stats.at(A) < Stats.at(B)
- : Stats.at(B) < Stats.at(A);
+ auto StatsItr = Stats.find(A);
+ assert(StatsItr != Stats.end());
+ const DynoStats &StatsA = StatsItr->second;
+
+ StatsItr = Stats.find(B);
+ assert(StatsItr != Stats.end());
+ const DynoStats &StatsB = StatsItr->second;
+
+ return Ascending ? StatsA < StatsB
+ : StatsB < StatsA;
});
} else {
llvm::stable_sort(
Functions, [Ascending, &Stats](const BinaryFunction *A,
const BinaryFunction *B) {
- const DynoStats &StatsA = Stats.at(A);
- const DynoStats &StatsB = Stats.at(B);
+ auto StatsItr = Stats.find(A);
+ assert(StatsItr != Stats.end());
+ const DynoStats &StatsA = StatsItr->second;
+
+ StatsItr = Stats.find(B);
+ assert(StatsItr != Stats.end());
+ const DynoStats &StatsB = StatsItr->second;
+
return Ascending ? StatsA.lessThan(StatsB, opts::PrintSortedBy)
: StatsB.lessThan(StatsA, opts::PrintSortedBy);
});
diff --git a/bolt/lib/Passes/CacheMetrics.cpp b/bolt/lib/Passes/CacheMetrics.cpp
index b02d4303110b3..7c710156c8346 100644
--- a/bolt/lib/Passes/CacheMetrics.cpp
+++ b/bolt/lib/Passes/CacheMetrics.cpp
@@ -67,7 +67,20 @@ calcTSPScore(const std::vector<BinaryFunction *> &BinaryFunctions,
for (BinaryBasicBlock *DstBB : SrcBB->successors()) {
if (SrcBB != DstBB && BI->Count != BinaryBasicBlock::COUNT_NO_PROFILE) {
JumpCount += BI->Count;
- if (BBAddr.at(SrcBB) + BBSize.at(SrcBB) == BBAddr.at(DstBB))
+
+ auto BBAddrIt = BBAddr.find(SrcBB);
+ assert(BBAddrIt != BBAddr.end());
+ uint64_t SrcBBAddr = BBAddrIt->second;
+
+ auto BBSizeIt = BBSize.find(SrcBB);
+ assert(BBSizeIt != BBSize.end());
+ uint64_t SrcBBSize = BBSizeIt->second;
+
+ BBAddrIt = BBAddr.find(DstBB);
+ assert(BBAddrIt != BBAddr.end());
+ uint64_t DstBBAddr = BBAddrIt->second;
+
+ if (SrcBBAddr + SrcBBSize == DstBBAddr)
Score += BI->Count;
}
++BI;
@@ -149,20 +162,30 @@ double expectedCacheHitRatio(
for (BinaryFunction *BF : BinaryFunctions) {
if (BF->getLayout().block_empty())
continue;
+ auto BBAddrIt = BBAddr.find(BF->getLayout().block_front());
+ assert(BBAddrIt != BBAddr.end());
const uint64_t Page =
- BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
- PageSamples[Page] += FunctionSamples.at(BF);
+ BBAddrIt->second / ITLBPageSize;
+
+ auto FunctionSamplesIt = FunctionSamples.find(BF);
+ assert(FunctionSamplesIt != FunctionSamples.end());
+ PageSamples[Page] += FunctionSamplesIt->second;
}
// Computing the expected number of misses for every function
double Misses = 0;
for (BinaryFunction *BF : BinaryFunctions) {
// Skip the function if it has no samples
- if (BF->getLayout().block_empty() || FunctionSamples.at(BF) == 0.0)
+ auto FunctionSamplesIt = FunctionSamples.find(BF);
+ assert(FunctionSamplesIt != FunctionSamples.end());
+ double Samples = FunctionSamplesIt->second;
+ if (BF->getLayout().block_empty() || Samples == 0.0)
continue;
- double Samples = FunctionSamples.at(BF);
+
+ auto BBAddrIt = BBAddr.find(BF->getLayout().block_front());
+ assert(BBAddrIt != BBAddr.end());
const uint64_t Page =
- BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
+ BBAddrIt->second / ITLBPageSize;
// The probability that the page is not present in the cache
const double MissProb =
pow(1.0 - PageSamples[Page] / TotalSamples, ITLBEntries);
@@ -170,8 +193,11 @@ double expectedCacheHitRatio(
// Processing all callers of the function
for (std::pair<BinaryFunction *, uint64_t> Pair : Calls[BF]) {
BinaryFunction *SrcFunction = Pair.first;
+
+ BBAddrIt = BBAddr.find(SrcFunction->getLayout().block_front());
+ assert(BBAddrIt != BBAddr.end());
const uint64_t SrcPage =
- BBAddr.at(SrcFunction->getLayout().block_front()) / ITLBPageSize;
+ BBAddrIt->second / ITLBPageSize;
// Is this a 'long' or a 'short' call?
if (Page != SrcPage) {
// This is a miss
diff --git a/bolt/lib/Passes/Inliner.cpp b/bolt/lib/Passes/Inliner.cpp
index 84e7d97067b0c..f004a8eeea185 100644
--- a/bolt/lib/Passes/Inliner.cpp
+++ b/bolt/lib/Passes/Inliner.cpp
@@ -355,7 +355,9 @@ Inliner::inlineCall(BinaryBasicBlock &CallerBB,
std::vector<BinaryBasicBlock *> Successors(BB.succ_size());
llvm::transform(BB.successors(), Successors.begin(),
[&InlinedBBMap](const BinaryBasicBlock *BB) {
- return InlinedBBMap.at(BB);
+ auto It = InlinedBBMap.find(BB);
+ assert(It != InlinedBBMap.end());
+ return It->second;
});
if (CallerFunction.hasValidProfile() && Callee.hasValidProfile())
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 016962ff34d8d..80c3c072e4ce6 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -372,8 +372,10 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
// Create necessary metadata for the flow function
for (FlowJump &Jump : Func.Jumps) {
- Func.Blocks.at(Jump.Source).SuccJumps.push_back(&Jump);
- Func.Blocks.at(Jump.Target).PredJumps.push_back(&Jump);
+ assert(Jump.Source < Func.Blocks.size());
+ Func.Blocks[Jump.Source].SuccJumps.push_back(&Jump);
+ assert(Jump.Target < Func.Blocks.size());
+ Func.Blocks[Jump.Target].PredJumps.push_back(&Jump);
}
return Func;
}
>From 6b44d15002ddbd13e5d883b41df70db38245c805 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 21 May 2024 12:22:51 -0700
Subject: [PATCH 2/3] Formatting
---
bolt/lib/Core/BinaryContext.cpp | 4 ++--
bolt/lib/Passes/BinaryPasses.cpp | 3 +--
bolt/lib/Passes/CacheMetrics.cpp | 9 +++------
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 046c5a3c0de06..494a09c933b4b 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -935,8 +935,8 @@ std::string BinaryContext::generateJumpTableName(const BinaryFunction &BF,
if (const JumpTable *JT = BF.getJumpTableContainingAddress(Address)) {
Offset = Address - JT->getAddress();
auto JTLabelsIt = JT->Labels.find(Offset);
- if ( JTLabelsIt != JT->Labels.end())
- return std::string(JTLabelsIt ->second->getName());
+ if (JTLabelsIt != JT->Labels.end())
+ return std::string(JTLabelsIt->second->getName());
auto JTIdsIt = JumpTableIds.find(JT->getAddress());
assert(JTIdsIt != JumpTableIds.end());
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index 713a909afe1a6..b733dacf6144a 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -1561,8 +1561,7 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
assert(StatsItr != Stats.end());
const DynoStats &StatsB = StatsItr->second;
- return Ascending ? StatsA < StatsB
- : StatsB < StatsA;
+ return Ascending ? StatsA < StatsB : StatsB < StatsA;
});
} else {
llvm::stable_sort(
diff --git a/bolt/lib/Passes/CacheMetrics.cpp b/bolt/lib/Passes/CacheMetrics.cpp
index 7c710156c8346..21b420a5c2b01 100644
--- a/bolt/lib/Passes/CacheMetrics.cpp
+++ b/bolt/lib/Passes/CacheMetrics.cpp
@@ -164,8 +164,7 @@ double expectedCacheHitRatio(
continue;
auto BBAddrIt = BBAddr.find(BF->getLayout().block_front());
assert(BBAddrIt != BBAddr.end());
- const uint64_t Page =
- BBAddrIt->second / ITLBPageSize;
+ const uint64_t Page = BBAddrIt->second / ITLBPageSize;
auto FunctionSamplesIt = FunctionSamples.find(BF);
assert(FunctionSamplesIt != FunctionSamples.end());
@@ -184,8 +183,7 @@ double expectedCacheHitRatio(
auto BBAddrIt = BBAddr.find(BF->getLayout().block_front());
assert(BBAddrIt != BBAddr.end());
- const uint64_t Page =
- BBAddrIt->second / ITLBPageSize;
+ const uint64_t Page = BBAddrIt->second / ITLBPageSize;
// The probability that the page is not present in the cache
const double MissProb =
pow(1.0 - PageSamples[Page] / TotalSamples, ITLBEntries);
@@ -196,8 +194,7 @@ double expectedCacheHitRatio(
BBAddrIt = BBAddr.find(SrcFunction->getLayout().block_front());
assert(BBAddrIt != BBAddr.end());
- const uint64_t SrcPage =
- BBAddrIt->second / ITLBPageSize;
+ const uint64_t SrcPage = BBAddrIt->second / ITLBPageSize;
// Is this a 'long' or a 'short' call?
if (Page != SrcPage) {
// This is a miss
>From df4a9e7f9582eb87d5b86d933cb6a720143f2c6b Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 21 May 2024 14:43:24 -0700
Subject: [PATCH 3/3] Construct function for comparing DynoStat objects
---
bolt/lib/Passes/BinaryPasses.cpp | 52 ++++++++++++++------------------
1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index e38dbbce61613..582cda9327dff 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -1553,36 +1553,28 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
const bool Ascending =
opts::DynoStatsSortOrderOpt == opts::DynoStatsSortOrder::Ascending;
- if (SortAll) {
- llvm::stable_sort(Functions,
- [Ascending, &Stats](const BinaryFunction *A,
- const BinaryFunction *B) {
- auto StatsItr = Stats.find(A);
- assert(StatsItr != Stats.end());
- const DynoStats &StatsA = StatsItr->second;
-
- StatsItr = Stats.find(B);
- assert(StatsItr != Stats.end());
- const DynoStats &StatsB = StatsItr->second;
-
- return Ascending ? StatsA < StatsB : StatsB < StatsA;
- });
- } else {
- llvm::stable_sort(
- Functions, [Ascending, &Stats](const BinaryFunction *A,
- const BinaryFunction *B) {
- auto StatsItr = Stats.find(A);
- assert(StatsItr != Stats.end());
- const DynoStats &StatsA = StatsItr->second;
-
- StatsItr = Stats.find(B);
- assert(StatsItr != Stats.end());
- const DynoStats &StatsB = StatsItr->second;
-
- return Ascending ? StatsA.lessThan(StatsB, opts::PrintSortedBy)
- : StatsB.lessThan(StatsA, opts::PrintSortedBy);
- });
- }
+ std::function<bool(const DynoStats &, const DynoStats &)>
+ DynoStatsComparator =
+ SortAll ? [](const DynoStats &StatsA,
+ const DynoStats &StatsB) { return StatsA < StatsB; }
+ : [](const DynoStats &StatsA, const DynoStats &StatsB) {
+ return StatsA.lessThan(StatsB, opts::PrintSortedBy);
+ };
+
+ llvm::stable_sort(Functions,
+ [Ascending, &Stats, DynoStatsComparator](
+ const BinaryFunction *A, const BinaryFunction *B) {
+ auto StatsItr = Stats.find(A);
+ assert(StatsItr != Stats.end());
+ const DynoStats &StatsA = StatsItr->second;
+
+ StatsItr = Stats.find(B);
+ assert(StatsItr != Stats.end());
+ const DynoStats &StatsB = StatsItr->second;
+
+ return Ascending ? DynoStatsComparator(StatsA, StatsB)
+ : DynoStatsComparator(StatsB, StatsA);
+ });
BC.outs() << "BOLT-INFO: top functions sorted by ";
if (SortAll) {
More information about the llvm-commits
mailing list