[llvm] [BOLT] Remove mutable from BB::LayoutIndex (PR #93224)
shaw young via llvm-commits
llvm-commits at lists.llvm.org
Fri May 31 09:45:21 PDT 2024
https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/93224
>From 4486b9c1a135fd1c758ac30379a133366fcd9917 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Thu, 23 May 2024 10:58:35 -0700
Subject: [PATCH 01/10] [BOLT][NFC] Remove mutable from BB:LayoutIndex
---
bolt/include/bolt/Core/BinaryBasicBlock.h | 4 ++--
bolt/lib/Core/BinaryFunction.cpp | 10 +++-------
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryBasicBlock.h b/bolt/include/bolt/Core/BinaryBasicBlock.h
index bc95e2c4de3a1..a57b70714fe38 100644
--- a/bolt/include/bolt/Core/BinaryBasicBlock.h
+++ b/bolt/include/bolt/Core/BinaryBasicBlock.h
@@ -115,7 +115,7 @@ class BinaryBasicBlock {
unsigned Index{InvalidIndex};
/// Index in the current layout.
- mutable unsigned LayoutIndex{InvalidIndex};
+ unsigned LayoutIndex{InvalidIndex};
/// Number of pseudo instructions in this block.
uint32_t NumPseudos{0};
@@ -891,7 +891,7 @@ class BinaryBasicBlock {
}
/// Set layout index. To be used by BinaryFunction.
- void setLayoutIndex(unsigned Index) const { LayoutIndex = Index; }
+ void setLayoutIndex(unsigned Index) { LayoutIndex = Index; }
/// Needed by graph traits.
BinaryFunction *getParent() const { return getFunction(); }
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 10b93e702984f..51dc5a2cbf0d6 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -3636,8 +3636,8 @@ bool BinaryFunction::forEachEntryPoint(EntryPointCallbackTy Callback) const {
BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
BasicBlockListType DFS;
- unsigned Index = 0;
std::stack<BinaryBasicBlock *> Stack;
+ std::set<BinaryBasicBlock *> Visited;
// Push entry points to the stack in reverse order.
//
@@ -3654,17 +3654,13 @@ BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
for (BinaryBasicBlock *const BB : reverse(EntryPoints))
Stack.push(BB);
- for (BinaryBasicBlock &BB : blocks())
- BB.setLayoutIndex(BinaryBasicBlock::InvalidIndex);
-
while (!Stack.empty()) {
BinaryBasicBlock *BB = Stack.top();
Stack.pop();
- if (BB->getLayoutIndex() != BinaryBasicBlock::InvalidIndex)
+ if (Visited.find(BB) != Visited.end())
continue;
-
- BB->setLayoutIndex(Index++);
+ Visited.insert(BB);
DFS.push_back(BB);
for (BinaryBasicBlock *SuccBB : BB->landing_pads()) {
>From c269abc0dc72bbe45b86a7ad79f8a68eb1ecef8d Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 24 May 2024 14:46:58 -0700
Subject: [PATCH 02/10] [BOLT][NFC] Fix invalid BB::getLayoutIndex calls
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 28 ++++++++++++++++++++++--
bolt/lib/Profile/YAMLProfileWriter.cpp | 13 +++++++++--
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index 87eba10354a37..e43c7e1a32c20 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -180,11 +180,26 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
const BinaryContext &BC = A.getBinaryContext();
+ std::unordered_map<const BinaryBasicBlock*, unsigned> LayoutIndiciesA;
+ for (unsigned I = 0; I < OrderA.size(); I++)
+ LayoutIndiciesA[OrderA[I]] = I;
+
+ std::unordered_map<const BinaryBasicBlock*, unsigned> LayoutIndiciesB;
+ for (unsigned I = 0; I < OrderB.size(); I++)
+ LayoutIndiciesB[OrderB[I]] = I;
+
auto BBI = OrderB.begin();
for (const BinaryBasicBlock *BB : OrderA) {
const BinaryBasicBlock *OtherBB = *BBI;
- if (BB->getLayoutIndex() != OtherBB->getLayoutIndex())
+ auto LayoutIndiciesAIt = LayoutIndiciesA.find(BB);
+ assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
+ unsigned BBLayoutIndex = LayoutIndiciesAIt->second;
+
+ auto LayoutIndiciesBIt = LayoutIndiciesB.find(OtherBB);
+ assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
+ unsigned OtherBBLayoutIndex = LayoutIndiciesBIt->second;
+ if (BBLayoutIndex != OtherBBLayoutIndex)
return false;
// Compare successor basic blocks.
@@ -195,7 +210,16 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
auto SuccBBI = OtherBB->succ_begin();
for (const BinaryBasicBlock *SuccBB : BB->successors()) {
const BinaryBasicBlock *SuccOtherBB = *SuccBBI;
- if (SuccBB->getLayoutIndex() != SuccOtherBB->getLayoutIndex())
+
+ LayoutIndiciesAIt = LayoutIndiciesA.find(SuccBB);
+ assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
+ unsigned SuccBBLayoutIndex = LayoutIndiciesAIt->second;
+
+ LayoutIndiciesBIt = LayoutIndiciesB.find(SuccOtherBB);
+ assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
+ unsigned SuccOtherBBLayoutIndex = LayoutIndiciesBIt->second;
+
+ if (SuccBBLayoutIndex != SuccOtherBBLayoutIndex)
return false;
++SuccBBI;
}
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index ef04ba0d21ad7..f0721a2a5d16b 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -69,9 +69,15 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));
+ std::unordered_map<const BinaryBasicBlock*, unsigned> LayoutIndicies;
+ for (unsigned I = 0; I < Order.size(); I++)
+ LayoutIndicies[Order[I]] = I;
+
for (const BinaryBasicBlock *BB : Order) {
yaml::bolt::BinaryBasicBlockProfile YamlBB;
- YamlBB.Index = BB->getLayoutIndex();
+ auto It = LayoutIndicies.find(BB);
+ assert(It != LayoutIndicies.end());
+ YamlBB.Index = It->second;
YamlBB.NumInstructions = BB->getNumNonPseudos();
YamlBB.Hash = BB->getHash();
@@ -160,7 +166,10 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
auto BranchInfo = BB->branch_info_begin();
for (const BinaryBasicBlock *Successor : BB->successors()) {
yaml::bolt::SuccessorInfo YamlSI;
- YamlSI.Index = Successor->getLayoutIndex();
+
+ auto It = LayoutIndicies.find(Successor);
+ assert(It != LayoutIndicies.end());
+ YamlSI.Index = It->second;
YamlSI.Count = BranchInfo->Count;
YamlSI.Mispreds = BranchInfo->MispredictedCount;
>From 44217bb26512fa6689e091185f76bb13eba9b154 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 24 May 2024 14:53:12 -0700
Subject: [PATCH 03/10] Formatting
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 4 ++--
bolt/lib/Profile/YAMLProfileWriter.cpp | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index e43c7e1a32c20..1c2f2470824f5 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -180,11 +180,11 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
const BinaryContext &BC = A.getBinaryContext();
- std::unordered_map<const BinaryBasicBlock*, unsigned> LayoutIndiciesA;
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesA;
for (unsigned I = 0; I < OrderA.size(); I++)
LayoutIndiciesA[OrderA[I]] = I;
- std::unordered_map<const BinaryBasicBlock*, unsigned> LayoutIndiciesB;
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesB;
for (unsigned I = 0; I < OrderB.size(); I++)
LayoutIndiciesB[OrderB[I]] = I;
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index f0721a2a5d16b..9aa2addec6c26 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -69,7 +69,7 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));
- std::unordered_map<const BinaryBasicBlock*, unsigned> LayoutIndicies;
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndicies;
for (unsigned I = 0; I < Order.size(); I++)
LayoutIndicies[Order[I]] = I;
>From 5fcb2604c6a3057b072397eb213ab52d0c5043e8 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 28 May 2024 09:30:30 -0700
Subject: [PATCH 04/10] [BOLT] Using dfs to initialize buckets in ICF
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D57867071
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 32 +++++-------------------
bolt/lib/Profile/YAMLProfileWriter.cpp | 12 ++-------
2 files changed, 8 insertions(+), 36 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index 1c2f2470824f5..d8b19bc7d3860 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -179,27 +179,11 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
}
const BinaryContext &BC = A.getBinaryContext();
-
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesA;
- for (unsigned I = 0; I < OrderA.size(); I++)
- LayoutIndiciesA[OrderA[I]] = I;
-
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesB;
- for (unsigned I = 0; I < OrderB.size(); I++)
- LayoutIndiciesB[OrderB[I]] = I;
-
auto BBI = OrderB.begin();
for (const BinaryBasicBlock *BB : OrderA) {
const BinaryBasicBlock *OtherBB = *BBI;
- auto LayoutIndiciesAIt = LayoutIndiciesA.find(BB);
- assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
- unsigned BBLayoutIndex = LayoutIndiciesAIt->second;
-
- auto LayoutIndiciesBIt = LayoutIndiciesB.find(OtherBB);
- assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
- unsigned OtherBBLayoutIndex = LayoutIndiciesBIt->second;
- if (BBLayoutIndex != OtherBBLayoutIndex)
+ if (BB->getLayoutIndex() != OtherBB->getLayoutIndex())
return false;
// Compare successor basic blocks.
@@ -211,15 +195,7 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
for (const BinaryBasicBlock *SuccBB : BB->successors()) {
const BinaryBasicBlock *SuccOtherBB = *SuccBBI;
- LayoutIndiciesAIt = LayoutIndiciesA.find(SuccBB);
- assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
- unsigned SuccBBLayoutIndex = LayoutIndiciesAIt->second;
-
- LayoutIndiciesBIt = LayoutIndiciesB.find(SuccOtherBB);
- assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
- unsigned SuccOtherBBLayoutIndex = LayoutIndiciesBIt->second;
-
- if (SuccBBLayoutIndex != SuccOtherBBLayoutIndex)
+ if (SuccBB->getLayoutIndex() != SuccOtherBB->getLayoutIndex())
return false;
++SuccBBI;
}
@@ -408,6 +384,8 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
BinaryFunction &BF = BFI.second;
if (!this->shouldOptimize(BF))
continue;
+ if (opts::ICFUseDFS)
+ BF.getLayout().update(BF.dfs());
CongruentBuckets[&BF].emplace(&BF);
}
};
@@ -433,6 +411,8 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
// Identical functions go into the same bucket.
IdenticalBucketsMap IdenticalBuckets;
for (BinaryFunction *BF : Candidates) {
+ if (opts::ICFUseDFS)
+ BF->getLayout().update(BF->dfs());
IdenticalBuckets[BF].emplace_back(BF);
}
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 9aa2addec6c26..d27d6f7a8164c 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -69,15 +69,9 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndicies;
- for (unsigned I = 0; I < Order.size(); I++)
- LayoutIndicies[Order[I]] = I;
-
for (const BinaryBasicBlock *BB : Order) {
yaml::bolt::BinaryBasicBlockProfile YamlBB;
- auto It = LayoutIndicies.find(BB);
- assert(It != LayoutIndicies.end());
- YamlBB.Index = It->second;
+ YamlBB.Index = BB->getLayoutIndex();
YamlBB.NumInstructions = BB->getNumNonPseudos();
YamlBB.Hash = BB->getHash();
@@ -167,9 +161,7 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
for (const BinaryBasicBlock *Successor : BB->successors()) {
yaml::bolt::SuccessorInfo YamlSI;
- auto It = LayoutIndicies.find(Successor);
- assert(It != LayoutIndicies.end());
- YamlSI.Index = It->second;
+ YamlSI.Index = Successor->getLayoutIndex();
YamlSI.Count = BranchInfo->Count;
YamlSI.Mispreds = BranchInfo->MispredictedCount;
>From c575fe578753ff7c5324288ae34d3895ddbfc671 Mon Sep 17 00:00:00 2001
From: shaw young <58664393+shawbyoung at users.noreply.github.com>
Date: Tue, 28 May 2024 13:02:25 -0700
Subject: [PATCH 05/10] Update new lines in IdenticalCodeFolding.cpp
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index d8b19bc7d3860..80f9d78c12682 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -179,6 +179,7 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
}
const BinaryContext &BC = A.getBinaryContext();
+
auto BBI = OrderB.begin();
for (const BinaryBasicBlock *BB : OrderA) {
const BinaryBasicBlock *OtherBB = *BBI;
@@ -194,7 +195,6 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
auto SuccBBI = OtherBB->succ_begin();
for (const BinaryBasicBlock *SuccBB : BB->successors()) {
const BinaryBasicBlock *SuccOtherBB = *SuccBBI;
-
if (SuccBB->getLayoutIndex() != SuccOtherBB->getLayoutIndex())
return false;
++SuccBBI;
>From 83ba4c40021c7e76b86a10428c214054a8f66f87 Mon Sep 17 00:00:00 2001
From: shaw young <58664393+shawbyoung at users.noreply.github.com>
Date: Tue, 28 May 2024 13:03:27 -0700
Subject: [PATCH 06/10] Update new lines in YAMLProfileWriter.cpp
---
bolt/lib/Profile/YAMLProfileWriter.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index d27d6f7a8164c..ef04ba0d21ad7 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -160,7 +160,6 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
auto BranchInfo = BB->branch_info_begin();
for (const BinaryBasicBlock *Successor : BB->successors()) {
yaml::bolt::SuccessorInfo YamlSI;
-
YamlSI.Index = Successor->getLayoutIndex();
YamlSI.Count = BranchInfo->Count;
YamlSI.Mispreds = BranchInfo->MispredictedCount;
>From cfa94b539eb86fd0afd40cc121d6024de2ffbf15 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Wed, 29 May 2024 08:38:01 -0700
Subject: [PATCH 07/10] [BOLT] Update and use appropriate LayoutIndices
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 5 ++++-
bolt/lib/Profile/YAMLProfileWriter.cpp | 13 ++++++++++---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index 80f9d78c12682..b20a175131855 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -356,7 +356,10 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
"ICF breakdown", opts::TimeICF);
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
// Make sure indices are in-order.
- BF.getLayout().updateLayoutIndices();
+ if (opts::ICFUseDFS)
+ BF.getLayout().update(BF.dfs());
+ else
+ BF.getLayout().updateLayoutIndices();
// Pre-compute hash before pushing into hashtable.
// Hash instruction operands to minimize hash collisions.
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index ef04ba0d21ad7..7c529413c6352 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -69,9 +69,15 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndices;
+ for (auto It : llvm::enumerate(Order))
+ LayoutIndices[It.value()] = It.index();
+
for (const BinaryBasicBlock *BB : Order) {
yaml::bolt::BinaryBasicBlockProfile YamlBB;
- YamlBB.Index = BB->getLayoutIndex();
+ auto LayoutIndicesIt = LayoutIndices.find(BB);
+ assert(LayoutIndicesIt != LayoutIndices.end());
+ YamlBB.Index = LayoutIndicesIt->second;
YamlBB.NumInstructions = BB->getNumNonPseudos();
YamlBB.Hash = BB->getHash();
@@ -160,10 +166,11 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
auto BranchInfo = BB->branch_info_begin();
for (const BinaryBasicBlock *Successor : BB->successors()) {
yaml::bolt::SuccessorInfo YamlSI;
- YamlSI.Index = Successor->getLayoutIndex();
+ LayoutIndicesIt = LayoutIndices.find(Successor);
+ assert(LayoutIndicesIt != LayoutIndices.end());
+ YamlSI.Index = LayoutIndicesIt->second;
YamlSI.Count = BranchInfo->Count;
YamlSI.Mispreds = BranchInfo->MispredictedCount;
-
YamlBB.Successors.emplace_back(YamlSI);
++BranchInfo;
>From dc07a21ad1a340f8d33759d76cebf1c719be9dd2 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Wed, 29 May 2024 08:38:01 -0700
Subject: [PATCH 08/10] [BOLT] No longer altering layout indices in ICF
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 80 ++++++++++++++++++------
bolt/lib/Profile/YAMLProfileWriter.cpp | 7 ++-
2 files changed, 65 insertions(+), 22 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index b20a175131855..a58d72cb43e4b 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -44,12 +44,17 @@ TimeICF("time-icf",
cl::cat(BoltOptCategory));
} // namespace opts
+using LayoutIndiciesMapTy =
+ std::unordered_map<const BinaryBasicBlock *, unsigned>;
+
/// Compare two jump tables in 2 functions. The function relies on consistent
/// ordering of basic blocks in both binary functions (e.g. DFS).
static bool equalJumpTables(const JumpTable &JumpTableA,
const JumpTable &JumpTableB,
const BinaryFunction &FunctionA,
- const BinaryFunction &FunctionB) {
+ const BinaryFunction &FunctionB,
+ const LayoutIndiciesMapTy &LayoutIndiciesA,
+ const LayoutIndiciesMapTy &LayoutIndiciesB) {
if (JumpTableA.EntrySize != JumpTableB.EntrySize)
return false;
@@ -80,7 +85,15 @@ static bool equalJumpTables(const JumpTable &JumpTableA,
assert(TargetA && TargetB && "cannot locate target block(s)");
- if (TargetA->getLayoutIndex() != TargetB->getLayoutIndex())
+ auto LayoutIndiciesAIt = LayoutIndiciesA.find(TargetA);
+ assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
+ unsigned TargetALayoutIndex = LayoutIndiciesAIt->second;
+
+ auto LayoutIndiciesBIt = LayoutIndiciesB.find(TargetB);
+ assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
+ unsigned TargetBLayoutIndex = LayoutIndiciesBIt->second;
+
+ if (TargetALayoutIndex != TargetBLayoutIndex)
return false;
}
@@ -91,10 +104,11 @@ static bool equalJumpTables(const JumpTable &JumpTableA,
/// given instruction of the given function. The functions should have
/// identical CFG.
template <class Compare>
-static bool isInstrEquivalentWith(const MCInst &InstA,
- const BinaryBasicBlock &BBA,
- const MCInst &InstB,
- const BinaryBasicBlock &BBB, Compare Comp) {
+static bool
+isInstrEquivalentWith(const MCInst &InstA, const BinaryBasicBlock &BBA,
+ const MCInst &InstB, const BinaryBasicBlock &BBB,
+ Compare Comp, const LayoutIndiciesMapTy &LayoutIndiciesA,
+ const LayoutIndiciesMapTy &LayoutIndiciesB) {
if (InstA.getOpcode() != InstB.getOpcode())
return false;
@@ -133,7 +147,15 @@ static bool isInstrEquivalentWith(const MCInst &InstA,
const BinaryBasicBlock *LPB = BBB.getLandingPad(EHInfoB->first);
assert(LPA && LPB && "cannot locate landing pad(s)");
- if (LPA->getLayoutIndex() != LPB->getLayoutIndex())
+ auto LayoutIndiciesAIt = LayoutIndiciesA.find(LPA);
+ assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
+ unsigned LPALayoutIndex = LayoutIndiciesAIt->second;
+
+ auto LayoutIndiciesBIt = LayoutIndiciesB.find(LPB);
+ assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
+ unsigned LPBLayoutIndex = LayoutIndiciesBIt->second;
+
+ if (LPALayoutIndex != LPBLayoutIndex)
return false;
}
}
@@ -177,6 +199,14 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
copy(A.getLayout().blocks(), std::back_inserter(OrderA));
copy(B.getLayout().blocks(), std::back_inserter(OrderB));
}
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesA(
+ OrderA.size());
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesB(
+ OrderB.size());
+ for (auto [Index, BB] : llvm::enumerate(OrderA))
+ LayoutIndiciesA[BB] = Index;
+ for (auto [Index, BB] : llvm::enumerate(OrderB))
+ LayoutIndiciesB[BB] = Index;
const BinaryContext &BC = A.getBinaryContext();
@@ -184,7 +214,15 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
for (const BinaryBasicBlock *BB : OrderA) {
const BinaryBasicBlock *OtherBB = *BBI;
- if (BB->getLayoutIndex() != OtherBB->getLayoutIndex())
+ auto LayoutIndiciesAIt = LayoutIndiciesA.find(BB);
+ assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
+ unsigned BBLayoutIndex = LayoutIndiciesAIt->second;
+
+ auto LayoutIndiciesBIt = LayoutIndiciesB.find(OtherBB);
+ assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
+ unsigned OtherBBLayoutIndex = LayoutIndiciesBIt->second;
+
+ if (BBLayoutIndex != OtherBBLayoutIndex)
return false;
// Compare successor basic blocks.
@@ -195,7 +233,16 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
auto SuccBBI = OtherBB->succ_begin();
for (const BinaryBasicBlock *SuccBB : BB->successors()) {
const BinaryBasicBlock *SuccOtherBB = *SuccBBI;
- if (SuccBB->getLayoutIndex() != SuccOtherBB->getLayoutIndex())
+
+ LayoutIndiciesAIt = LayoutIndiciesA.find(SuccBB);
+ assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
+ unsigned SuccBBLayoutIndex = LayoutIndiciesAIt->second;
+
+ LayoutIndiciesBIt = LayoutIndiciesB.find(SuccOtherBB);
+ assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
+ unsigned SuccOtherBBLayoutIndex = LayoutIndiciesBIt->second;
+
+ if (SuccBBLayoutIndex != SuccOtherBBLayoutIndex)
return false;
++SuccBBI;
}
@@ -271,11 +318,13 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
(SIB->getAddress() - JumpTableB->getAddress()))
return false;
- return equalJumpTables(*JumpTableA, *JumpTableB, A, B);
+ return equalJumpTables(*JumpTableA, *JumpTableB, A, B, LayoutIndiciesA,
+ LayoutIndiciesB);
};
if (!isInstrEquivalentWith(*I, *BB, *OtherI, *OtherBB,
- AreSymbolsIdentical))
+ AreSymbolsIdentical, LayoutIndiciesA,
+ LayoutIndiciesB))
return false;
++I;
@@ -356,10 +405,7 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
"ICF breakdown", opts::TimeICF);
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
// Make sure indices are in-order.
- if (opts::ICFUseDFS)
- BF.getLayout().update(BF.dfs());
- else
- BF.getLayout().updateLayoutIndices();
+ BF.getLayout().updateLayoutIndices();
// Pre-compute hash before pushing into hashtable.
// Hash instruction operands to minimize hash collisions.
@@ -387,8 +433,6 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
BinaryFunction &BF = BFI.second;
if (!this->shouldOptimize(BF))
continue;
- if (opts::ICFUseDFS)
- BF.getLayout().update(BF.dfs());
CongruentBuckets[&BF].emplace(&BF);
}
};
@@ -414,8 +458,6 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
// Identical functions go into the same bucket.
IdenticalBucketsMap IdenticalBuckets;
for (BinaryFunction *BF : Candidates) {
- if (opts::ICFUseDFS)
- BF->getLayout().update(BF->dfs());
IdenticalBuckets[BF].emplace_back(BF);
}
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 7c529413c6352..f5f022e0d5905 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -69,9 +69,10 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndices;
- for (auto It : llvm::enumerate(Order))
- LayoutIndices[It.value()] = It.index();
+ std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndices(
+ Order.size());
+ for (auto [Index, BB] : llvm::enumerate(Order))
+ LayoutIndices[BB] = Index;
for (const BinaryBasicBlock *BB : Order) {
yaml::bolt::BinaryBasicBlockProfile YamlBB;
>From 194ef06e0213712678b0233b3f204967d69693eb Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 31 May 2024 09:25:50 -0700
Subject: [PATCH 09/10] Set LayoutIndices in ICF and YAMLProfileWriter
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 72 ++++--------------------
bolt/lib/Profile/YAMLProfileWriter.cpp | 14 ++---
2 files changed, 16 insertions(+), 70 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index a58d72cb43e4b..53c2f92cc7c1e 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -44,17 +44,12 @@ TimeICF("time-icf",
cl::cat(BoltOptCategory));
} // namespace opts
-using LayoutIndiciesMapTy =
- std::unordered_map<const BinaryBasicBlock *, unsigned>;
-
/// Compare two jump tables in 2 functions. The function relies on consistent
/// ordering of basic blocks in both binary functions (e.g. DFS).
static bool equalJumpTables(const JumpTable &JumpTableA,
const JumpTable &JumpTableB,
const BinaryFunction &FunctionA,
- const BinaryFunction &FunctionB,
- const LayoutIndiciesMapTy &LayoutIndiciesA,
- const LayoutIndiciesMapTy &LayoutIndiciesB) {
+ const BinaryFunction &FunctionB) {
if (JumpTableA.EntrySize != JumpTableB.EntrySize)
return false;
@@ -84,16 +79,7 @@ static bool equalJumpTables(const JumpTable &JumpTableA,
}
assert(TargetA && TargetB && "cannot locate target block(s)");
-
- auto LayoutIndiciesAIt = LayoutIndiciesA.find(TargetA);
- assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
- unsigned TargetALayoutIndex = LayoutIndiciesAIt->second;
-
- auto LayoutIndiciesBIt = LayoutIndiciesB.find(TargetB);
- assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
- unsigned TargetBLayoutIndex = LayoutIndiciesBIt->second;
-
- if (TargetALayoutIndex != TargetBLayoutIndex)
+ if (TargetA->getLayoutIndex() != TargetB->getLayoutIndex())
return false;
}
@@ -107,8 +93,7 @@ template <class Compare>
static bool
isInstrEquivalentWith(const MCInst &InstA, const BinaryBasicBlock &BBA,
const MCInst &InstB, const BinaryBasicBlock &BBB,
- Compare Comp, const LayoutIndiciesMapTy &LayoutIndiciesA,
- const LayoutIndiciesMapTy &LayoutIndiciesB) {
+ Compare Comp) {
if (InstA.getOpcode() != InstB.getOpcode())
return false;
@@ -147,15 +132,7 @@ isInstrEquivalentWith(const MCInst &InstA, const BinaryBasicBlock &BBA,
const BinaryBasicBlock *LPB = BBB.getLandingPad(EHInfoB->first);
assert(LPA && LPB && "cannot locate landing pad(s)");
- auto LayoutIndiciesAIt = LayoutIndiciesA.find(LPA);
- assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
- unsigned LPALayoutIndex = LayoutIndiciesAIt->second;
-
- auto LayoutIndiciesBIt = LayoutIndiciesB.find(LPB);
- assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
- unsigned LPBLayoutIndex = LayoutIndiciesBIt->second;
-
- if (LPALayoutIndex != LPBLayoutIndex)
+ if (LPA->getLayoutIndex() != LPB->getLayoutIndex())
return false;
}
}
@@ -199,30 +176,13 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
copy(A.getLayout().blocks(), std::back_inserter(OrderA));
copy(B.getLayout().blocks(), std::back_inserter(OrderB));
}
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesA(
- OrderA.size());
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndiciesB(
- OrderB.size());
- for (auto [Index, BB] : llvm::enumerate(OrderA))
- LayoutIndiciesA[BB] = Index;
- for (auto [Index, BB] : llvm::enumerate(OrderB))
- LayoutIndiciesB[BB] = Index;
const BinaryContext &BC = A.getBinaryContext();
auto BBI = OrderB.begin();
for (const BinaryBasicBlock *BB : OrderA) {
const BinaryBasicBlock *OtherBB = *BBI;
-
- auto LayoutIndiciesAIt = LayoutIndiciesA.find(BB);
- assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
- unsigned BBLayoutIndex = LayoutIndiciesAIt->second;
-
- auto LayoutIndiciesBIt = LayoutIndiciesB.find(OtherBB);
- assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
- unsigned OtherBBLayoutIndex = LayoutIndiciesBIt->second;
-
- if (BBLayoutIndex != OtherBBLayoutIndex)
+ if (BB->getLayoutIndex() != OtherBB->getLayoutIndex())
return false;
// Compare successor basic blocks.
@@ -233,16 +193,7 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
auto SuccBBI = OtherBB->succ_begin();
for (const BinaryBasicBlock *SuccBB : BB->successors()) {
const BinaryBasicBlock *SuccOtherBB = *SuccBBI;
-
- LayoutIndiciesAIt = LayoutIndiciesA.find(SuccBB);
- assert(LayoutIndiciesAIt != LayoutIndiciesA.end());
- unsigned SuccBBLayoutIndex = LayoutIndiciesAIt->second;
-
- LayoutIndiciesBIt = LayoutIndiciesB.find(SuccOtherBB);
- assert(LayoutIndiciesBIt != LayoutIndiciesB.end());
- unsigned SuccOtherBBLayoutIndex = LayoutIndiciesBIt->second;
-
- if (SuccBBLayoutIndex != SuccOtherBBLayoutIndex)
+ if (SuccBB->getLayoutIndex() != SuccOtherBB->getLayoutIndex())
return false;
++SuccBBI;
}
@@ -318,13 +269,11 @@ static bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
(SIB->getAddress() - JumpTableB->getAddress()))
return false;
- return equalJumpTables(*JumpTableA, *JumpTableB, A, B, LayoutIndiciesA,
- LayoutIndiciesB);
+ return equalJumpTables(*JumpTableA, *JumpTableB, A, B);
};
if (!isInstrEquivalentWith(*I, *BB, *OtherI, *OtherBB,
- AreSymbolsIdentical, LayoutIndiciesA,
- LayoutIndiciesB))
+ AreSymbolsIdentical))
return false;
++I;
@@ -405,7 +354,10 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
"ICF breakdown", opts::TimeICF);
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
// Make sure indices are in-order.
- BF.getLayout().updateLayoutIndices();
+ if (opts::ICFUseDFS)
+ BF.getLayout().updateLayoutIndices(BF.dfs());
+ else
+ BF.getLayout().updateLayoutIndices();
// Pre-compute hash before pushing into hashtable.
// Hash instruction operands to minimize hash collisions.
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 1acf7ede1b659..cfce6e643885e 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -74,16 +74,12 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));
- std::unordered_map<const BinaryBasicBlock *, unsigned> LayoutIndices(
- Order.size());
- for (auto [Index, BB] : llvm::enumerate(Order))
- LayoutIndices[BB] = Index;
+ const FunctionLayout Layout = BF.getLayout();
+ Layout.updateLayoutIndices(Order);
for (const BinaryBasicBlock *BB : Order) {
yaml::bolt::BinaryBasicBlockProfile YamlBB;
- auto LayoutIndicesIt = LayoutIndices.find(BB);
- assert(LayoutIndicesIt != LayoutIndices.end());
- YamlBB.Index = LayoutIndicesIt->second;
+ YamlBB.Index = BB->getLayoutIndex();
YamlBB.NumInstructions = BB->getNumNonPseudos();
YamlBB.Hash = BB->getHash();
@@ -172,9 +168,7 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
auto BranchInfo = BB->branch_info_begin();
for (const BinaryBasicBlock *Successor : BB->successors()) {
yaml::bolt::SuccessorInfo YamlSI;
- LayoutIndicesIt = LayoutIndices.find(Successor);
- assert(LayoutIndicesIt != LayoutIndices.end());
- YamlSI.Index = LayoutIndicesIt->second;
+ YamlSI.Index = Successor->getLayoutIndex();
YamlSI.Count = BranchInfo->Count;
YamlSI.Mispreds = BranchInfo->MispredictedCount;
YamlBB.Successors.emplace_back(YamlSI);
>From 371181b355b206b3245af0ca7affdad3f50a07bd Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Fri, 31 May 2024 09:44:50 -0700
Subject: [PATCH 10/10] Formatting
---
bolt/lib/Passes/IdenticalCodeFolding.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/bolt/lib/Passes/IdenticalCodeFolding.cpp b/bolt/lib/Passes/IdenticalCodeFolding.cpp
index 53c2f92cc7c1e..6829f0c622a14 100644
--- a/bolt/lib/Passes/IdenticalCodeFolding.cpp
+++ b/bolt/lib/Passes/IdenticalCodeFolding.cpp
@@ -90,10 +90,10 @@ static bool equalJumpTables(const JumpTable &JumpTableA,
/// given instruction of the given function. The functions should have
/// identical CFG.
template <class Compare>
-static bool
-isInstrEquivalentWith(const MCInst &InstA, const BinaryBasicBlock &BBA,
- const MCInst &InstB, const BinaryBasicBlock &BBB,
- Compare Comp) {
+static bool isInstrEquivalentWith(const MCInst &InstA,
+ const BinaryBasicBlock &BBA,
+ const MCInst &InstB,
+ const BinaryBasicBlock &BBB, Compare Comp) {
if (InstA.getOpcode() != InstB.getOpcode())
return false;
More information about the llvm-commits
mailing list