[llvm] [BOLT][NFC] Add sink block to flow CFG in profile inference (PR #95047)
shaw young via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 09:16:30 PDT 2024
https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/95047
>From 1b984ba9747618e277c8ffb2c48467aec7c2a6a3 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 10 Jun 2024 12:08:38 -0700
Subject: [PATCH 1/6] [BOLT][NFC] Add sink block to flow CFG in profile
inference
Test Plan: tbd
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D58380996
---
bolt/lib/Profile/StaleProfileMatching.cpp | 38 ++++++++++++++++---
.../Transforms/Utils/SampleProfileInference.h | 3 +-
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 365bc5389266d..8ecfb618072ab 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -309,22 +309,33 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
FlowFunction Func;
// Add a special "dummy" source so that there is always a unique entry point.
- // Because of the extra source, for all other blocks in FlowFunction it holds
- // that Block.Index == BB->getIndex() + 1
FlowBlock EntryBlock;
EntryBlock.Index = 0;
Func.Blocks.push_back(EntryBlock);
+ auto BinaryBlockIsExit = [&](const BinaryBasicBlock &BB) {
+ if (BB.successors().empty())
+ return true;
+ return false;
+ };
+
// Create FlowBlock for every basic block in the binary function
for (const BinaryBasicBlock *BB : BlockOrder) {
Func.Blocks.emplace_back();
FlowBlock &Block = Func.Blocks.back();
Block.Index = Func.Blocks.size() - 1;
+ Block.HasSuccessors = BinaryBlockIsExit(*BB);
(void)BB;
assert(Block.Index == BB->getIndex() + 1 &&
"incorrectly assigned basic block index");
}
+ // Add a special "dummy" sink block so there is always a unique sink
+ FlowBlock SinkBlock;
+ SinkBlock.Index = Func.Blocks.size();
+ Func.Blocks.push_back(SinkBlock);
+ Func.Sink = SinkBlock.Index;
+
// Create FlowJump for each jump between basic blocks in the binary function
std::vector<uint64_t> InDegree(Func.Blocks.size(), 0);
for (const BinaryBasicBlock *SrcBB : BlockOrder) {
@@ -360,18 +371,29 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
// Add dummy edges to the extra sources. If there are multiple entry blocks,
// add an unlikely edge from 0 to the subsequent ones
assert(InDegree[0] == 0 && "dummy entry blocks shouldn't have predecessors");
- for (uint64_t I = 1; I < Func.Blocks.size(); I++) {
+ for (uint64_t I = 1; I < BlockOrder.size() + 1; I++) {
const BinaryBasicBlock *BB = BlockOrder[I - 1];
if (BB->isEntryPoint() || InDegree[I] == 0) {
Func.Jumps.emplace_back();
FlowJump &Jump = Func.Jumps.back();
- Jump.Source = 0;
+ Jump.Source = Func.Entry;
Jump.Target = I;
if (!BB->isEntryPoint())
Jump.IsUnlikely = true;
}
}
+ // Add dummy edges from the exit blocks to the sink block.
+ for (uint64_t I = 1; I < BlockOrder.size() + 1; I++) {
+ FlowBlock &Block = Func.Blocks[I];
+ if (Block.HasSuccessors) {
+ Func.Jumps.emplace_back();
+ FlowJump &Jump = Func.Jumps.back();
+ Jump.Source = I;
+ Jump.Target = Func.Sink;
+ }
+ }
+
// Create necessary metadata for the flow function
for (FlowJump &Jump : Func.Jumps) {
assert(Jump.Source < Func.Blocks.size());
@@ -379,6 +401,7 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
assert(Jump.Target < Func.Blocks.size());
Func.Blocks[Jump.Target].PredJumps.push_back(&Jump);
}
+
return Func;
}
@@ -395,7 +418,7 @@ void matchWeightsByHashes(BinaryContext &BC,
const BinaryFunction::BasicBlockOrderType &BlockOrder,
const yaml::bolt::BinaryFunctionProfile &YamlBF,
FlowFunction &Func) {
- assert(Func.Blocks.size() == BlockOrder.size() + 1);
+ assert(Func.Blocks.size() == BlockOrder.size() + 2);
std::vector<FlowBlock *> Blocks;
std::vector<BlendedBlockHash> BlendedHashes;
@@ -618,7 +641,7 @@ void assignProfile(BinaryFunction &BF,
FlowFunction &Func) {
BinaryContext &BC = BF.getBinaryContext();
- assert(Func.Blocks.size() == BlockOrder.size() + 1);
+ assert(Func.Blocks.size() == BlockOrder.size() + 2);
for (uint64_t I = 0; I < BlockOrder.size(); I++) {
FlowBlock &Block = Func.Blocks[I + 1];
BinaryBasicBlock *BB = BlockOrder[I];
@@ -640,6 +663,9 @@ void assignProfile(BinaryFunction &BF,
if (Jump->Flow == 0)
continue;
+ // Skip the artificial sink block
+ if (Jump->Target == Func.Sink)
+ continue;
BinaryBasicBlock &SuccBB = *BlockOrder[Jump->Target - 1];
// Check if the edge corresponds to a regular jump or a landing pad
if (BB->getSuccessor(SuccBB.getLabel())) {
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
index b4ea1ad840f9d..b2af05a24c705 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -31,10 +31,10 @@ struct FlowBlock {
uint64_t Flow{0};
std::vector<FlowJump *> SuccJumps;
std::vector<FlowJump *> PredJumps;
+ bool HasSuccessors{false};
/// Check if it is the entry block in the function.
bool isEntry() const { return PredJumps.empty(); }
-
/// Check if it is an exit block in the function.
bool isExit() const { return SuccJumps.empty(); }
};
@@ -57,6 +57,7 @@ struct FlowFunction {
std::vector<FlowJump> Jumps;
/// The index of the entry block.
uint64_t Entry{0};
+ uint64_t Sink{0};
};
/// Various thresholds and options controlling the behavior of the profile
>From 34922ba752ebe755e89fdf3cca0ef58073e7a9b0 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 10 Jun 2024 16:21:36 -0700
Subject: [PATCH 2/6] Changing design decisions
---
bolt/lib/Profile/StaleProfileMatching.cpp | 18 ++++++------------
.../Transforms/Utils/SampleProfileInference.h | 5 +++--
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 8ecfb618072ab..58f9f459af84a 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -313,18 +313,13 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
EntryBlock.Index = 0;
Func.Blocks.push_back(EntryBlock);
- auto BinaryBlockIsExit = [&](const BinaryBasicBlock &BB) {
- if (BB.successors().empty())
- return true;
- return false;
- };
-
// Create FlowBlock for every basic block in the binary function
for (const BinaryBasicBlock *BB : BlockOrder) {
Func.Blocks.emplace_back();
FlowBlock &Block = Func.Blocks.back();
Block.Index = Func.Blocks.size() - 1;
- Block.HasSuccessors = BinaryBlockIsExit(*BB);
+ if (BB->successors().empty())
+ Block.IsExit = true;
(void)BB;
assert(Block.Index == BB->getIndex() + 1 &&
"incorrectly assigned basic block index");
@@ -369,14 +364,14 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
}
// Add dummy edges to the extra sources. If there are multiple entry blocks,
- // add an unlikely edge from 0 to the subsequent ones
+ // add an unlikely edge from 0 to the subsequent ones. Skips the sink block.
assert(InDegree[0] == 0 && "dummy entry blocks shouldn't have predecessors");
- for (uint64_t I = 1; I < BlockOrder.size() + 1; I++) {
+ for (uint64_t I = 1; I < Func.Blocks.size() - 1; I++) {
const BinaryBasicBlock *BB = BlockOrder[I - 1];
if (BB->isEntryPoint() || InDegree[I] == 0) {
Func.Jumps.emplace_back();
FlowJump &Jump = Func.Jumps.back();
- Jump.Source = Func.Entry;
+ Jump.Source = 0;
Jump.Target = I;
if (!BB->isEntryPoint())
Jump.IsUnlikely = true;
@@ -386,7 +381,7 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
// Add dummy edges from the exit blocks to the sink block.
for (uint64_t I = 1; I < BlockOrder.size() + 1; I++) {
FlowBlock &Block = Func.Blocks[I];
- if (Block.HasSuccessors) {
+ if (Block.IsExit) {
Func.Jumps.emplace_back();
FlowJump &Jump = Func.Jumps.back();
Jump.Source = I;
@@ -401,7 +396,6 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
assert(Jump.Target < Func.Blocks.size());
Func.Blocks[Jump.Target].PredJumps.push_back(&Jump);
}
-
return Func;
}
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
index b2af05a24c705..1f74dc0f7108b 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -28,13 +28,14 @@ struct FlowBlock {
uint64_t Weight{0};
bool HasUnknownWeight{true};
bool IsUnlikely{false};
+ bool IsExit{false};
uint64_t Flow{0};
std::vector<FlowJump *> SuccJumps;
std::vector<FlowJump *> PredJumps;
- bool HasSuccessors{false};
-
+
/// Check if it is the entry block in the function.
bool isEntry() const { return PredJumps.empty(); }
+
/// Check if it is an exit block in the function.
bool isExit() const { return SuccJumps.empty(); }
};
>From 1712ecb7c9bb9e1c381221a4bd34f44b17590ef2 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Mon, 10 Jun 2024 16:31:11 -0700
Subject: [PATCH 3/6] Removing whitespace
---
llvm/include/llvm/Transforms/Utils/SampleProfileInference.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
index 1f74dc0f7108b..d8962a76a8016 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -32,7 +32,7 @@ struct FlowBlock {
uint64_t Flow{0};
std::vector<FlowJump *> SuccJumps;
std::vector<FlowJump *> PredJumps;
-
+
/// Check if it is the entry block in the function.
bool isEntry() const { return PredJumps.empty(); }
>From 78493cbff5ae141cc6b388ca9dcb306597566bbd Mon Sep 17 00:00:00 2001
From: shaw young <58664393+shawbyoung at users.noreply.github.com>
Date: Mon, 10 Jun 2024 17:00:19 -0700
Subject: [PATCH 4/6] Update bolt/lib/Profile/StaleProfileMatching.cpp
Co-authored-by: Amir Ayupov <fads93 at gmail.com>
---
bolt/lib/Profile/StaleProfileMatching.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 58f9f459af84a..5e3e9e8e725b1 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -318,8 +318,7 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
Func.Blocks.emplace_back();
FlowBlock &Block = Func.Blocks.back();
Block.Index = Func.Blocks.size() - 1;
- if (BB->successors().empty())
- Block.IsExit = true;
+ Block.IsExit = BB->successors().empty();
(void)BB;
assert(Block.Index == BB->getIndex() + 1 &&
"incorrectly assigned basic block index");
>From 4f647bf3a12b8fb876d6784f00830e84aa3e7d95 Mon Sep 17 00:00:00 2001
From: shawbyoung <shawbyoung at gmail.com>
Date: Tue, 11 Jun 2024 00:49:48 -0700
Subject: [PATCH 5/6] Suggested style changes
---
bolt/lib/Profile/StaleProfileMatching.cpp | 20 +++++++++----------
.../Transforms/Utils/SampleProfileInference.h | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 58f9f459af84a..5969f4b9bcbc1 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -313,7 +313,7 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
EntryBlock.Index = 0;
Func.Blocks.push_back(EntryBlock);
- // Create FlowBlock for every basic block in the binary function
+ // Create FlowBlock for every basic block in the binary function.
for (const BinaryBasicBlock *BB : BlockOrder) {
Func.Blocks.emplace_back();
FlowBlock &Block = Func.Blocks.back();
@@ -325,13 +325,13 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
"incorrectly assigned basic block index");
}
- // Add a special "dummy" sink block so there is always a unique sink
+ // Add a special "dummy" sink block so there is always a unique sink.
FlowBlock SinkBlock;
SinkBlock.Index = Func.Blocks.size();
Func.Blocks.push_back(SinkBlock);
Func.Sink = SinkBlock.Index;
- // Create FlowJump for each jump between basic blocks in the binary function
+ // Create FlowJump for each jump between basic blocks in the binary function.
std::vector<uint64_t> InDegree(Func.Blocks.size(), 0);
for (const BinaryBasicBlock *SrcBB : BlockOrder) {
std::unordered_set<const BinaryBasicBlock *> UniqueSuccs;
@@ -380,13 +380,13 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
// Add dummy edges from the exit blocks to the sink block.
for (uint64_t I = 1; I < BlockOrder.size() + 1; I++) {
- FlowBlock &Block = Func.Blocks[I];
- if (Block.IsExit) {
- Func.Jumps.emplace_back();
- FlowJump &Jump = Func.Jumps.back();
- Jump.Source = I;
- Jump.Target = Func.Sink;
- }
+ if (!Func.Blocks[I].IsExit)
+ continue;
+
+ Func.Jumps.emplace_back();
+ FlowJump &Jump = Func.Jumps.back();
+ Jump.Source = I;
+ Jump.Target = Func.Sink;
}
// Create necessary metadata for the flow function
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
index d8962a76a8016..70a80b91405d0 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -58,7 +58,7 @@ struct FlowFunction {
std::vector<FlowJump> Jumps;
/// The index of the entry block.
uint64_t Entry{0};
- uint64_t Sink{0};
+ uint64_t Sink{UINT64_MAX};
};
/// Various thresholds and options controlling the behavior of the profile
>From 4fc35430a0be5afb4ff7909ef6f51859b7019e33 Mon Sep 17 00:00:00 2001
From: shaw young <58664393+shawbyoung at users.noreply.github.com>
Date: Tue, 11 Jun 2024 09:16:20 -0700
Subject: [PATCH 6/6] Commenting
---
bolt/lib/Profile/StaleProfileMatching.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index cf75b3da0e58d..a35513abbefc4 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -656,7 +656,7 @@ void assignProfile(BinaryFunction &BF,
if (Jump->Flow == 0)
continue;
- // Skip the artificial sink block
+ // Skips the artificial sink block.
if (Jump->Target == Func.Sink)
continue;
BinaryBasicBlock &SuccBB = *BlockOrder[Jump->Target - 1];
More information about the llvm-commits
mailing list