Skip to content

Commit c694368

Browse files
FatihBAKIRFatihBAKIR
committed
Refactor hot path pass. The fall through conversion is moved to a free
function is clearer. Handles errors properly, and upon a failure, emits a warning and skips the erroneous path.
1 parent b9a2c15 commit c694368

File tree

1 file changed

+111
-70
lines changed

1 file changed

+111
-70
lines changed

Diff for: ‎llvm/lib/CodeGen/HotPath.cpp

+111-70
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,94 @@ MachineBasicBlock *CloneMachineBasicBlock(MachineBasicBlock &bb) {
3131
return MBB;
3232
}
3333

34+
// Converts the path from the from block to the to block to be a fallthrough.
35+
// Requires to to be a successor of from.
36+
// to_block must be placed after from_block in the layout after this call!
37+
// Returns true if the conversion could not be completed.
38+
// If the conversion fails, the parameters will not have changed.
39+
bool ConvertToFallthrough(const TargetInstrInfo *TII,
40+
MachineBasicBlock *from_block,
41+
const MachineBasicBlock *to_block) {
42+
assert(from_block->isSuccessor(to_block) &&
43+
"The given block must be a successor of from block.");
44+
MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
45+
SmallVector<MachineOperand, 4> Cond;
46+
47+
if (TII->analyzeBranch(*from_block, TBB, FBB, Cond)) {
48+
return true;
49+
}
50+
51+
if (!TBB && !FBB) {
52+
// Already falls through, no need to modify the block.
53+
return false;
54+
}
55+
56+
if (TBB && !FBB && Cond.empty()) {
57+
// The from_block has an unconditional jump at the end.
58+
// We need to remove that branch so it falls through.
59+
60+
assert(TBB == to_block &&
61+
"from_block ends with an unconditional jump, and to_block is it's "
62+
"successor, the jump must be to to_block.");
63+
64+
TII->removeBranch(*from_block);
65+
return false;
66+
}
67+
68+
if (TBB && !FBB) {
69+
// There's a conditional jump to a block. It could be jumping to the
70+
// original block, or it could be falling through to the original
71+
// block.
72+
73+
if (TBB == to_block) {
74+
// Jumps to original block. We need to make this fall-through to us.
75+
// Need to invert the branch, make it jump to it's current fall
76+
// through and fall through to us.
77+
if (TII->reverseBranchCondition(Cond)) {
78+
// Could not reverse the condition, abort?
79+
return true;
80+
}
81+
82+
auto current_fallthrough = from_block->getFallThrough();
83+
TII->removeBranch(*from_block);
84+
TII->insertBranch(*from_block, current_fallthrough, nullptr, Cond,
85+
from_block->findBranchDebugLoc());
86+
} else {
87+
// Already falls through, no need to modify.
88+
}
89+
return false;
90+
}
91+
92+
if (TBB && FBB) {
93+
// The conditional has jump instructions in either direction. We can
94+
// eliminate one of the jumps and make it fall through to us.
95+
96+
if (TBB == to_block) {
97+
// Make the true case fall through.
98+
if (TII->reverseBranchCondition(Cond)) {
99+
// Could not reverse the condition.
100+
return true;
101+
}
102+
103+
// We will remove this branch and generate a new one to TBB below.
104+
// Since TBB is the block we want to fall through to, after reversing
105+
// the branch condition, we also swap the true and false branches.
106+
std::swap(FBB, TBB);
107+
} else {
108+
// Make the false case fall through. This is trivial to do.
109+
assert(FBB == to_block &&
110+
"to_block is a successor, but it is neither the true basic block, "
111+
"nor the false basic block.");
112+
}
113+
114+
TII->removeBranch(*from_block);
115+
TII->insertBranch(*from_block, TBB, nullptr, Cond,
116+
from_block->findBranchDebugLoc());
117+
}
118+
119+
llvm_unreachable("All cases are handled.");
120+
}
121+
34122
class HotPath : public MachineFunctionPass {
35123
public:
36124
static char ID;
@@ -40,8 +128,11 @@ class HotPath : public MachineFunctionPass {
40128
}
41129

42130
bool runOnMachineFunction(MachineFunction &MF) override {
43-
if (MF.getName().find("do_work") != llvm::StringRef::npos) {
131+
auto TII = MF.getSubtarget().getInstrInfo();
44132

133+
// The function name and the path to generate is hard coded. It will be
134+
// read from the llvm profile data.
135+
if (MF.getName().find("do_work") != llvm::StringRef::npos) {
45136
using HotPathBlocks = std::vector<MachineBasicBlock *>;
46137

47138
HotPathBlocks hotpath;
@@ -50,88 +141,37 @@ class HotPath : public MachineFunctionPass {
50141

51142
std::vector<HotPathBlocks> paths{std::move(hotpath)};
52143

53-
auto TII = MF.getSubtarget().getInstrInfo();
54-
55144
for (auto &hot_path : paths) {
56145
auto block = hot_path[1]; // The middle block is to be cloned.
57146

58-
auto cloned = CloneMachineBasicBlock(*block);
59-
60-
// Add the successors of the original block as the new block's
61-
// successors as well.
62-
for (auto succ : block->successors()) {
63-
cloned->addSuccessorWithoutProb(succ);
64-
}
65-
66147
// Remove the original block from the successors of the previous block.
67148
// The remove call removes pred_block from predecessors of block as
68149
// well.
69150
auto pred_block = hot_path[0];
70151
auto layout_succ = pred_block->getFallThrough();
71-
pred_block->removeSuccessor(block);
72152

73-
// Add the block as a successor to the previous block in the hot path.
74-
pred_block->addSuccessor(cloned, BranchProbability::getOne());
153+
if (ConvertToFallthrough(TII, pred_block, block)) {
154+
WithColor::warning() << "Hot path generation failed.";
155+
continue;
156+
}
75157

76-
MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
77-
SmallVector<MachineOperand, 4> Cond;
158+
// The pred_block falls through to block now,
159+
pred_block->removeSuccessor(block);
78160

79-
if (TII->analyzeBranch(*pred_block, TBB, FBB, Cond)) {
80-
llvm_unreachable("Could not analyze branch");
81-
}
161+
auto cloned = CloneMachineBasicBlock(*block);
82162

83-
if (!TBB && !FBB) {
84-
// Falls through to it's successor, no need to modify the block.
85-
} else if (TBB && !FBB && Cond.empty()) {
86-
if (TBB != block) {
87-
llvm_unreachable("We were the successor of this block, it should "
88-
"be jumping to us");
89-
}
90-
// The pred_block has an unconditional jump to the original block.
91-
// We need to remove that branch so it falls through to us.
92-
TII->removeBranch(*pred_block);
93-
} else if (TBB && !FBB) {
94-
// There's a conditional jump to a block. It could be jumping to the
95-
// original block, or it could be falling through to the original
96-
// block.
97-
if (TBB == block) {
98-
// Jumps to original block. We need to make this fall-through to us.
99-
// Need to invert the branch, make it jump to it's current fall
100-
// through and fall through to us.
101-
if (!TII->reverseBranchCondition(Cond)) {
102-
auto current_fallthrough = pred_block->getFallThrough();
103-
TII->removeBranch(*pred_block);
104-
TII->insertBranch(*pred_block, current_fallthrough, nullptr, Cond,
105-
pred_block->findBranchDebugLoc());
106-
} else {
107-
// Could not reverse the condition, abort?
108-
llvm_unreachable("");
109-
}
110-
} else {
111-
// Falls through to original block, no need to modify.
112-
}
113-
} else if (TBB && FBB) {
114-
// The conditional has jump instructions in either direction. We can
115-
// eliminate one of the jumps and make it fall through to us.
116-
117-
if (TBB == block) {
118-
// Make the true case fall through.
119-
if (!TII->reverseBranchCondition(Cond)) {
120-
std::swap(FBB, TBB);
121-
} else {
122-
// Could not reverse the condition, abort?
123-
llvm_unreachable("");
124-
}
125-
} else {
126-
// Make the false case fall through. This is trivial to do.
127-
assert(FBB == block);
128-
}
129-
130-
TII->removeBranch(*pred_block);
131-
TII->insertBranch(*pred_block, TBB, nullptr, Cond,
132-
pred_block->findBranchDebugLoc());
163+
// Add the successors of the original block as the new block's
164+
// successors as well.
165+
auto succ_end = block->succ_end();
166+
for (auto succ_it = block->succ_begin(); succ_it != succ_end;
167+
++succ_it) {
168+
cloned->copySuccessor(block, succ_it);
133169
}
134170

171+
// Add the block as a successor to the previous block in the hot path.
172+
// TODO: get this probability from the profile.
173+
pred_block->addSuccessor(cloned, BranchProbability::getOne());
174+
135175
// The pred block always falls through to us.
136176
cloned->moveAfter(pred_block);
137177

@@ -146,7 +186,8 @@ class HotPath : public MachineFunctionPass {
146186
cloned->findBranchDebugLoc());
147187
}
148188

149-
assert(pred_block->getFallThrough() == cloned);
189+
assert(pred_block->getFallThrough() == cloned &&
190+
"Hot path pass did not generate a fall-through path!");
150191

151192
for (auto &live : block->liveins()) {
152193
cloned->addLiveIn(live);

0 commit comments

Comments
 (0)