Skip to content

Commit abea99f

Browse files
author
Quentin Colombet
committed
[MachineDominatorTree] Provide a method to inform a MachineDominatorTree that a
critical edge has been split. The MachineDominatorTree will when lazy update the underlying dominance properties when require. ** Context ** This is a follow-up of r215410. Each time a critical edge is split this invalidates the dominator tree information. Thus, subsequent queries of that interface will be slow until the underlying information is actually recomputed (costly). ** Problem ** Prior to this patch, splitting a critical edge needed to query the dominator tree to update the dominator information. Therefore, splitting a bunch of critical edges will likely produce poor performance as each query to the dominator tree will use the slow query path. This happens a lot in passes like MachineSink and PHIElimination. ** Proposed Solution ** Splitting a critical edge is a local modification of the CFG. Moreover, as soon as a critical edge is split, it is not critical anymore and thus cannot be a candidate for critical edge splitting anymore. In other words, the predecessor and successor of a basic block inserted on a critical edge cannot be inserted by critical edge splitting. Using these observations, we can pile up the splitting of critical edge and apply then at once before updating the DT information. The core of this patch moves the update of the MachineDominatorTree information from MachineBasicBlock::SplitCriticalEdge to a lazy MachineDominatorTree. ** Performance ** Thanks to this patch, the motivating example compiles in 4- minutes instead of 6+ minutes. No test case added as the motivating example as nothing special but being huge! The binaries are strictly identical for all the llvm test-suite + SPECs with and without this patch for both Os and O3. Regarding compile time, I observed only noise, although on average I saw a small improvement. <rdar://problem/17894619> llvm-svn: 215576
1 parent 46d1544 commit abea99f

File tree

3 files changed

+145
-26
lines changed

3 files changed

+145
-26
lines changed

llvm/include/llvm/CodeGen/MachineDominators.h

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#ifndef LLVM_CODEGEN_MACHINEDOMINATORS_H
1616
#define LLVM_CODEGEN_MACHINEDOMINATORS_H
1717

18+
#include "llvm/ADT/SmallSet.h"
1819
#include "llvm/CodeGen/MachineBasicBlock.h"
1920
#include "llvm/CodeGen/MachineFunction.h"
2021
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -38,6 +39,103 @@ typedef DomTreeNodeBase<MachineBasicBlock> MachineDomTreeNode;
3839
/// compute a normal dominator tree.
3940
///
4041
class MachineDominatorTree : public MachineFunctionPass {
42+
/// \brief Helper structure used to hold all the basic blocks
43+
/// involved in the split of a critical edge.
44+
struct CriticalEdge {
45+
MachineBasicBlock *FromBB;
46+
MachineBasicBlock *ToBB;
47+
MachineBasicBlock *NewBB;
48+
CriticalEdge(MachineBasicBlock *FromBB, MachineBasicBlock *ToBB,
49+
MachineBasicBlock *NewBB)
50+
: FromBB(FromBB), ToBB(ToBB), NewBB(NewBB) {}
51+
};
52+
53+
/// \brief Pile up all the critical edges to be split.
54+
/// The splitting of a critical edge is local and thus, it is possible
55+
/// to apply several of those changes at the same time.
56+
mutable SmallVector<CriticalEdge, 32> CriticalEdgesToSplit;
57+
/// \brief Remember all the basic blocks that are inserted during
58+
/// edge splitting.
59+
/// Invariant: NewBBs == all the basic blocks contained in the NewBB
60+
/// field of all the elements of CriticalEdgesToSplit.
61+
/// I.e., forall elt in CriticalEdgesToSplit, it exists BB in NewBBs
62+
/// such as BB == elt.NewBB.
63+
mutable SmallSet<MachineBasicBlock *, 32> NewBBs;
64+
65+
/// \brief Apply all the recorded critical edges to the DT.
66+
/// This updates the underlying DT information in a way that uses
67+
/// the fast query path of DT as much as possible.
68+
///
69+
/// \post CriticalEdgesToSplit.empty().
70+
void applySplitCriticalEdges() const {
71+
// Bail out early if there is nothing to do.
72+
if (CriticalEdgesToSplit.empty())
73+
return;
74+
75+
// For each element in CriticalEdgesToSplit, remember whether or
76+
// not element is the new immediate domminator of its successor.
77+
// The mapping is done by index, i.e., the information for the ith
78+
// element of CriticalEdgesToSplit is the ith element of IsNewIDom.
79+
SmallVector<bool, 32> IsNewIDom;
80+
IsNewIDom.resize(CriticalEdgesToSplit.size());
81+
size_t Idx = 0;
82+
83+
// Collect all the dominance properties info, before invalidating
84+
// the underlying DT.
85+
for (CriticalEdge &Edge : CriticalEdgesToSplit) {
86+
// Update dominator information.
87+
MachineBasicBlock *Succ = Edge.ToBB;
88+
MachineDomTreeNode *SucccDTNode = DT->getNode(Succ);
89+
90+
IsNewIDom[Idx] = true;
91+
for (MachineBasicBlock *PredBB : Succ->predecessors()) {
92+
if (PredBB == Edge.NewBB)
93+
continue;
94+
// If we are in this situation:
95+
// FromBB1 FromBB2
96+
// + +
97+
// + + + +
98+
// + + + +
99+
// ... Split1 Split2 ...
100+
// + +
101+
// + +
102+
// +
103+
// Succ
104+
// Instead of checking the domiance property with Split2, we
105+
// check it with FromBB2 since Split2 is still unknown of the
106+
// underlying DT structure.
107+
if (NewBBs.count(PredBB)) {
108+
assert(PredBB->pred_size() == 1 && "A basic block resulting from a "
109+
"critical edge split has more "
110+
"than one predecessor!");
111+
PredBB = *PredBB->pred_begin();
112+
}
113+
if (!DT->dominates(SucccDTNode, DT->getNode(PredBB))) {
114+
IsNewIDom[Idx] = false;
115+
break;
116+
}
117+
}
118+
++Idx;
119+
}
120+
121+
// Now, update DT with the collected dominance properties info.
122+
Idx = 0;
123+
for (CriticalEdge &Edge : CriticalEdgesToSplit) {
124+
// We know FromBB dominates NewBB.
125+
MachineDomTreeNode *NewDTNode = DT->addNewBlock(Edge.NewBB, Edge.FromBB);
126+
MachineDomTreeNode *SucccDTNode = DT->getNode(Edge.ToBB);
127+
128+
// If all the other predecessors of "Succ" are dominated by "Succ" itself
129+
// then the new block is the new immediate dominator of "Succ". Otherwise,
130+
// the new block doesn't dominate anything.
131+
if (IsNewIDom[Idx])
132+
DT->changeImmediateDominator(SucccDTNode, NewDTNode);
133+
++Idx;
134+
}
135+
NewBBs.clear();
136+
CriticalEdgesToSplit.clear();
137+
}
138+
41139
public:
42140
static char ID; // Pass ID, replacement for typeid
43141
DominatorTreeBase<MachineBasicBlock>* DT;
@@ -46,7 +144,10 @@ class MachineDominatorTree : public MachineFunctionPass {
46144

47145
~MachineDominatorTree();
48146

49-
DominatorTreeBase<MachineBasicBlock>& getBase() { return *DT; }
147+
DominatorTreeBase<MachineBasicBlock> &getBase() {
148+
applySplitCriticalEdges();
149+
return *DT;
150+
}
50151

51152
void getAnalysisUsage(AnalysisUsage &AU) const override;
52153

@@ -55,32 +156,38 @@ class MachineDominatorTree : public MachineFunctionPass {
55156
/// dominators, this will always be a single block (the entry node).
56157
///
57158
inline const std::vector<MachineBasicBlock*> &getRoots() const {
159+
applySplitCriticalEdges();
58160
return DT->getRoots();
59161
}
60162

61163
inline MachineBasicBlock *getRoot() const {
164+
applySplitCriticalEdges();
62165
return DT->getRoot();
63166
}
64167

65168
inline MachineDomTreeNode *getRootNode() const {
169+
applySplitCriticalEdges();
66170
return DT->getRootNode();
67171
}
68172

69173
bool runOnMachineFunction(MachineFunction &F) override;
70174

71175
inline bool dominates(const MachineDomTreeNode* A,
72176
const MachineDomTreeNode* B) const {
177+
applySplitCriticalEdges();
73178
return DT->dominates(A, B);
74179
}
75180

76181
inline bool dominates(const MachineBasicBlock* A,
77182
const MachineBasicBlock* B) const {
183+
applySplitCriticalEdges();
78184
return DT->dominates(A, B);
79185
}
80186

81187
// dominates - Return true if A dominates B. This performs the
82188
// special checks necessary if A and B are in the same basic block.
83189
bool dominates(const MachineInstr *A, const MachineInstr *B) const {
190+
applySplitCriticalEdges();
84191
const MachineBasicBlock *BBA = A->getParent(), *BBB = B->getParent();
85192
if (BBA != BBB) return DT->dominates(BBA, BBB);
86193

@@ -100,29 +207,34 @@ class MachineDominatorTree : public MachineFunctionPass {
100207

101208
inline bool properlyDominates(const MachineDomTreeNode* A,
102209
const MachineDomTreeNode* B) const {
210+
applySplitCriticalEdges();
103211
return DT->properlyDominates(A, B);
104212
}
105213

106214
inline bool properlyDominates(const MachineBasicBlock* A,
107215
const MachineBasicBlock* B) const {
216+
applySplitCriticalEdges();
108217
return DT->properlyDominates(A, B);
109218
}
110219

111220
/// findNearestCommonDominator - Find nearest common dominator basic block
112221
/// for basic block A and B. If there is no such block then return NULL.
113222
inline MachineBasicBlock *findNearestCommonDominator(MachineBasicBlock *A,
114223
MachineBasicBlock *B) {
224+
applySplitCriticalEdges();
115225
return DT->findNearestCommonDominator(A, B);
116226
}
117227

118228
inline MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
229+
applySplitCriticalEdges();
119230
return DT->getNode(BB);
120231
}
121232

122233
/// getNode - return the (Post)DominatorTree node for the specified basic
123234
/// block. This is the same as using operator[] on this class.
124235
///
125236
inline MachineDomTreeNode *getNode(MachineBasicBlock *BB) const {
237+
applySplitCriticalEdges();
126238
return DT->getNode(BB);
127239
}
128240

@@ -131,6 +243,7 @@ class MachineDominatorTree : public MachineFunctionPass {
131243
/// the children list of the immediate dominator.
132244
inline MachineDomTreeNode *addNewBlock(MachineBasicBlock *BB,
133245
MachineBasicBlock *DomBB) {
246+
applySplitCriticalEdges();
134247
return DT->addNewBlock(BB, DomBB);
135248
}
136249

@@ -139,36 +252,63 @@ class MachineDominatorTree : public MachineFunctionPass {
139252
///
140253
inline void changeImmediateDominator(MachineBasicBlock *N,
141254
MachineBasicBlock* NewIDom) {
255+
applySplitCriticalEdges();
142256
DT->changeImmediateDominator(N, NewIDom);
143257
}
144258

145259
inline void changeImmediateDominator(MachineDomTreeNode *N,
146260
MachineDomTreeNode* NewIDom) {
261+
applySplitCriticalEdges();
147262
DT->changeImmediateDominator(N, NewIDom);
148263
}
149264

150265
/// eraseNode - Removes a node from the dominator tree. Block must not
151266
/// dominate any other blocks. Removes node from its immediate dominator's
152267
/// children list. Deletes dominator node associated with basic block BB.
153268
inline void eraseNode(MachineBasicBlock *BB) {
269+
applySplitCriticalEdges();
154270
DT->eraseNode(BB);
155271
}
156272

157273
/// splitBlock - BB is split and now it has one successor. Update dominator
158274
/// tree to reflect this change.
159275
inline void splitBlock(MachineBasicBlock* NewBB) {
276+
applySplitCriticalEdges();
160277
DT->splitBlock(NewBB);
161278
}
162279

163280
/// isReachableFromEntry - Return true if A is dominated by the entry
164281
/// block of the function containing it.
165282
bool isReachableFromEntry(const MachineBasicBlock *A) {
283+
applySplitCriticalEdges();
166284
return DT->isReachableFromEntry(A);
167285
}
168286

169287
void releaseMemory() override;
170288

171289
void print(raw_ostream &OS, const Module*) const override;
290+
291+
/// \brief Record that the critical edge (FromBB, ToBB) has been
292+
/// split with NewBB.
293+
/// This is best to use this method instead of directly update the
294+
/// underlying information, because this helps mitigating the
295+
/// number of time the DT information is invalidated.
296+
///
297+
/// \note Do not use this method with regular edges.
298+
///
299+
/// \note To benefit from the compile time improvement incurred by this
300+
/// method, the users of this method have to limit the queries to the DT
301+
/// interface between two edges splitting. In other words, they have to
302+
/// pack the splitting of critical edges as much as possible.
303+
void recordSplitCriticalEdge(MachineBasicBlock *FromBB,
304+
MachineBasicBlock *ToBB,
305+
MachineBasicBlock *NewBB) {
306+
bool Inserted = NewBBs.insert(NewBB);
307+
(void)Inserted;
308+
assert(Inserted &&
309+
"A basic block inserted via edge splitting cannot appear twice");
310+
CriticalEdgesToSplit.push_back(CriticalEdge(FromBB, ToBB, NewBB));
311+
}
172312
};
173313

174314
//===-------------------------------------

llvm/lib/CodeGen/MachineBasicBlock.cpp

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -906,31 +906,8 @@ MachineBasicBlock::SplitCriticalEdge(MachineBasicBlock *Succ, Pass *P) {
906906
}
907907

908908
if (MachineDominatorTree *MDT =
909-
P->getAnalysisIfAvailable<MachineDominatorTree>()) {
910-
// Update dominator information.
911-
MachineDomTreeNode *SucccDTNode = MDT->getNode(Succ);
912-
913-
bool IsNewIDom = true;
914-
for (const_pred_iterator PI = Succ->pred_begin(), E = Succ->pred_end();
915-
PI != E; ++PI) {
916-
MachineBasicBlock *PredBB = *PI;
917-
if (PredBB == NMBB)
918-
continue;
919-
if (!MDT->dominates(SucccDTNode, MDT->getNode(PredBB))) {
920-
IsNewIDom = false;
921-
break;
922-
}
923-
}
924-
925-
// We know "this" dominates the newly created basic block.
926-
MachineDomTreeNode *NewDTNode = MDT->addNewBlock(NMBB, this);
927-
928-
// If all the other predecessors of "Succ" are dominated by "Succ" itself
929-
// then the new block is the new immediate dominator of "Succ". Otherwise,
930-
// the new block doesn't dominate anything.
931-
if (IsNewIDom)
932-
MDT->changeImmediateDominator(SucccDTNode, NewDTNode);
933-
}
909+
P->getAnalysisIfAvailable<MachineDominatorTree>())
910+
MDT->recordSplitCriticalEdge(this, Succ, NMBB);
934911

935912
if (MachineLoopInfo *MLI = P->getAnalysisIfAvailable<MachineLoopInfo>())
936913
if (MachineLoop *TIL = MLI->getLoopFor(this)) {

llvm/lib/CodeGen/MachineDominators.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ void MachineDominatorTree::getAnalysisUsage(AnalysisUsage &AU) const {
3535
}
3636

3737
bool MachineDominatorTree::runOnMachineFunction(MachineFunction &F) {
38+
CriticalEdgesToSplit.clear();
39+
NewBBs.clear();
3840
DT->recalculate(F);
3941

4042
return false;

0 commit comments

Comments
 (0)