Skip to content

Commit 6a90769

Browse files
committed
Revert "[CodeGen] Remove applySplitCriticalEdges in MachineDominatorTree (#97055)"
This reverts commit c5e5088. Causes large compile-time regressions.
1 parent 7868033 commit 6a90769

21 files changed

+284
-44
lines changed

llvm/include/llvm/CodeGen/MachineDominators.h

Lines changed: 167 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,86 @@ extern template bool Verify<MBBDomTree>(const MBBDomTree &DT,
7373
/// compute a normal dominator tree.
7474
///
7575
class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
76+
/// Helper structure used to hold all the basic blocks
77+
/// involved in the split of a critical edge.
78+
struct CriticalEdge {
79+
MachineBasicBlock *FromBB;
80+
MachineBasicBlock *ToBB;
81+
MachineBasicBlock *NewBB;
82+
};
83+
84+
/// Pile up all the critical edges to be split.
85+
/// The splitting of a critical edge is local and thus, it is possible
86+
/// to apply several of those changes at the same time.
87+
mutable SmallVector<CriticalEdge, 32> CriticalEdgesToSplit;
88+
89+
/// Remember all the basic blocks that are inserted during
90+
/// edge splitting.
91+
/// Invariant: NewBBs == all the basic blocks contained in the NewBB
92+
/// field of all the elements of CriticalEdgesToSplit.
93+
/// I.e., forall elt in CriticalEdgesToSplit, it exists BB in NewBBs
94+
/// such as BB == elt.NewBB.
95+
mutable SmallSet<MachineBasicBlock *, 32> NewBBs;
96+
97+
/// Apply all the recorded critical edges to the DT.
98+
/// This updates the underlying DT information in a way that uses
99+
/// the fast query path of DT as much as possible.
100+
/// FIXME: This method should not be a const member!
101+
///
102+
/// \post CriticalEdgesToSplit.empty().
103+
void applySplitCriticalEdges() const;
76104

77105
public:
78106
using Base = DomTreeBase<MachineBasicBlock>;
79107

80108
MachineDominatorTree() = default;
81-
explicit MachineDominatorTree(MachineFunction &MF) { recalculate(MF); }
109+
explicit MachineDominatorTree(MachineFunction &MF) { calculate(MF); }
82110

83111
/// Handle invalidation explicitly.
84112
bool invalidate(MachineFunction &, const PreservedAnalyses &PA,
85113
MachineFunctionAnalysisManager::Invalidator &);
86114

87-
using Base::dominates;
115+
// FIXME: If there is an updater for MachineDominatorTree,
116+
// migrate to this updater and remove these wrappers.
117+
118+
MachineDominatorTree &getBase() {
119+
applySplitCriticalEdges();
120+
return *this;
121+
}
122+
123+
MachineBasicBlock *getRoot() const {
124+
applySplitCriticalEdges();
125+
return Base::getRoot();
126+
}
127+
128+
MachineDomTreeNode *getRootNode() const {
129+
applySplitCriticalEdges();
130+
return const_cast<MachineDomTreeNode *>(Base::getRootNode());
131+
}
132+
133+
void calculate(MachineFunction &F);
134+
135+
bool dominates(const MachineDomTreeNode *A,
136+
const MachineDomTreeNode *B) const {
137+
applySplitCriticalEdges();
138+
return Base::dominates(A, B);
139+
}
140+
141+
void getDescendants(MachineBasicBlock *A,
142+
SmallVectorImpl<MachineBasicBlock *> &Result) {
143+
applySplitCriticalEdges();
144+
Base::getDescendants(A, Result);
145+
}
146+
147+
bool dominates(const MachineBasicBlock *A, const MachineBasicBlock *B) const {
148+
applySplitCriticalEdges();
149+
return Base::dominates(A, B);
150+
}
88151

89152
// dominates - Return true if A dominates B. This performs the
90153
// special checks necessary if A and B are in the same basic block.
91154
bool dominates(const MachineInstr *A, const MachineInstr *B) const {
155+
applySplitCriticalEdges();
92156
const MachineBasicBlock *BBA = A->getParent(), *BBB = B->getParent();
93157
if (BBA != BBB)
94158
return Base::dominates(BBA, BBB);
@@ -100,6 +164,107 @@ class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
100164

101165
return &*I == A;
102166
}
167+
168+
bool properlyDominates(const MachineDomTreeNode *A,
169+
const MachineDomTreeNode *B) const {
170+
applySplitCriticalEdges();
171+
return Base::properlyDominates(A, B);
172+
}
173+
174+
bool properlyDominates(const MachineBasicBlock *A,
175+
const MachineBasicBlock *B) const {
176+
applySplitCriticalEdges();
177+
return Base::properlyDominates(A, B);
178+
}
179+
180+
/// findNearestCommonDominator - Find nearest common dominator basic block
181+
/// for basic block A and B. If there is no such block then return NULL.
182+
MachineBasicBlock *findNearestCommonDominator(MachineBasicBlock *A,
183+
MachineBasicBlock *B) {
184+
applySplitCriticalEdges();
185+
return Base::findNearestCommonDominator(A, B);
186+
}
187+
188+
MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
189+
applySplitCriticalEdges();
190+
return Base::getNode(BB);
191+
}
192+
193+
/// getNode - return the (Post)DominatorTree node for the specified basic
194+
/// block. This is the same as using operator[] on this class.
195+
///
196+
MachineDomTreeNode *getNode(MachineBasicBlock *BB) const {
197+
applySplitCriticalEdges();
198+
return Base::getNode(BB);
199+
}
200+
201+
/// addNewBlock - Add a new node to the dominator tree information. This
202+
/// creates a new node as a child of DomBB dominator node,linking it into
203+
/// the children list of the immediate dominator.
204+
MachineDomTreeNode *addNewBlock(MachineBasicBlock *BB,
205+
MachineBasicBlock *DomBB) {
206+
applySplitCriticalEdges();
207+
return Base::addNewBlock(BB, DomBB);
208+
}
209+
210+
/// changeImmediateDominator - This method is used to update the dominator
211+
/// tree information when a node's immediate dominator changes.
212+
///
213+
void changeImmediateDominator(MachineBasicBlock *N,
214+
MachineBasicBlock *NewIDom) {
215+
applySplitCriticalEdges();
216+
Base::changeImmediateDominator(N, NewIDom);
217+
}
218+
219+
void changeImmediateDominator(MachineDomTreeNode *N,
220+
MachineDomTreeNode *NewIDom) {
221+
applySplitCriticalEdges();
222+
Base::changeImmediateDominator(N, NewIDom);
223+
}
224+
225+
/// eraseNode - Removes a node from the dominator tree. Block must not
226+
/// dominate any other blocks. Removes node from its immediate dominator's
227+
/// children list. Deletes dominator node associated with basic block BB.
228+
void eraseNode(MachineBasicBlock *BB) {
229+
applySplitCriticalEdges();
230+
Base::eraseNode(BB);
231+
}
232+
233+
/// splitBlock - BB is split and now it has one successor. Update dominator
234+
/// tree to reflect this change.
235+
void splitBlock(MachineBasicBlock* NewBB) {
236+
applySplitCriticalEdges();
237+
Base::splitBlock(NewBB);
238+
}
239+
240+
/// isReachableFromEntry - Return true if A is dominated by the entry
241+
/// block of the function containing it.
242+
bool isReachableFromEntry(const MachineBasicBlock *A) {
243+
applySplitCriticalEdges();
244+
return Base::isReachableFromEntry(A);
245+
}
246+
247+
/// Record that the critical edge (FromBB, ToBB) has been
248+
/// split with NewBB.
249+
/// This is best to use this method instead of directly update the
250+
/// underlying information, because this helps mitigating the
251+
/// number of time the DT information is invalidated.
252+
///
253+
/// \note Do not use this method with regular edges.
254+
///
255+
/// \note To benefit from the compile time improvement incurred by this
256+
/// method, the users of this method have to limit the queries to the DT
257+
/// interface between two edges splitting. In other words, they have to
258+
/// pack the splitting of critical edges as much as possible.
259+
void recordSplitCriticalEdge(MachineBasicBlock *FromBB,
260+
MachineBasicBlock *ToBB,
261+
MachineBasicBlock *NewBB) {
262+
bool Inserted = NewBBs.insert(NewBB).second;
263+
(void)Inserted;
264+
assert(Inserted &&
265+
"A basic block inserted via edge splitting cannot appear twice");
266+
CriticalEdgesToSplit.push_back({FromBB, ToBB, NewBB});
267+
}
103268
};
104269

105270
/// \brief Analysis pass which computes a \c MachineDominatorTree.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,7 +1698,7 @@ void AsmPrinter::emitFunctionBody() {
16981698
MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
16991699
if (!MDT) {
17001700
OwnedMDT = std::make_unique<MachineDominatorTree>();
1701-
OwnedMDT->recalculate(*MF);
1701+
OwnedMDT->getBase().recalculate(*MF);
17021702
MDT = OwnedMDT.get();
17031703
}
17041704

@@ -1707,7 +1707,7 @@ void AsmPrinter::emitFunctionBody() {
17071707
MLI = MLIWrapper ? &MLIWrapper->getLI() : nullptr;
17081708
if (!MLI) {
17091709
OwnedMLI = std::make_unique<MachineLoopInfo>();
1710-
OwnedMLI->analyze(*MDT);
1710+
OwnedMLI->analyze(MDT->getBase());
17111711
MLI = OwnedMLI.get();
17121712
}
17131713
}

llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,13 @@ LazyMachineBlockFrequencyInfoPass::calculateIfNotAvailable() const {
7878
if (!MDT) {
7979
LLVM_DEBUG(dbgs() << "Building DominatorTree on the fly\n");
8080
OwnedMDT = std::make_unique<MachineDominatorTree>();
81-
OwnedMDT->recalculate(*MF);
81+
OwnedMDT->getBase().recalculate(*MF);
8282
MDT = OwnedMDT.get();
8383
}
8484

8585
// Generate LoopInfo from it.
8686
OwnedMLI = std::make_unique<MachineLoopInfo>();
87-
OwnedMLI->analyze(*MDT);
87+
OwnedMLI->analyze(MDT->getBase());
8888
MLI = OwnedMLI.get();
8989
}
9090

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2754,7 +2754,7 @@ void InstrRefBasedLDV::BlockPHIPlacement(
27542754
// Apply IDF calculator to the designated set of location defs, storing
27552755
// required PHIs into PHIBlocks. Uses the dominator tree stored in the
27562756
// InstrRefBasedLDV object.
2757-
IDFCalculatorBase<MachineBasicBlock, false> IDF(*DomTree);
2757+
IDFCalculatorBase<MachineBasicBlock, false> IDF(DomTree->getBase());
27582758

27592759
IDF.setLiveInBlocks(AllBlocks);
27602760
IDF.setDefiningBlocks(DefBlocks);

llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) {
120120
MachineDominatorTree *DomTree = nullptr;
121121
if (InstrRefBased) {
122122
DomTree = &MDT;
123-
MDT.recalculate(MF);
123+
MDT.calculate(MF);
124124
TheImpl = &*InstrRefImpl;
125125
}
126126

llvm/lib/CodeGen/MachineBasicBlock.cpp

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@
1616
#include "llvm/CodeGen/LiveIntervals.h"
1717
#include "llvm/CodeGen/LivePhysRegs.h"
1818
#include "llvm/CodeGen/LiveVariables.h"
19-
#include "llvm/CodeGen/MachineDomTreeUpdater.h"
2019
#include "llvm/CodeGen/MachineDominators.h"
2120
#include "llvm/CodeGen/MachineFunction.h"
2221
#include "llvm/CodeGen/MachineInstrBuilder.h"
2322
#include "llvm/CodeGen/MachineJumpTableInfo.h"
2423
#include "llvm/CodeGen/MachineLoopInfo.h"
25-
#include "llvm/CodeGen/MachinePostDominators.h"
2624
#include "llvm/CodeGen/MachineRegisterInfo.h"
2725
#include "llvm/CodeGen/SlotIndexes.h"
2826
#include "llvm/CodeGen/TargetInstrInfo.h"
@@ -1341,17 +1339,9 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
13411339
LIS->repairIntervalsInRange(this, getFirstTerminator(), end(), UsedRegs);
13421340
}
13431341

1344-
auto *MDTWrapper =
1345-
P.getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
1346-
auto *MPDTWrapper =
1347-
P.getAnalysisIfAvailable<MachinePostDominatorTreeWrapperPass>();
1348-
auto *MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
1349-
auto *MPDT = MPDTWrapper ? &MPDTWrapper->getPostDomTree() : nullptr;
1350-
MachineDomTreeUpdater MDTU(MDT, MPDT,
1351-
MachineDomTreeUpdater::UpdateStrategy::Eager);
1352-
MDTU.applyUpdates({{MachineDominatorTree::Insert, this, NMBB},
1353-
{MachineDominatorTree::Insert, NMBB, Succ},
1354-
{MachineDominatorTree::Delete, this, Succ}});
1342+
if (auto *MDTWrapper =
1343+
P.getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>())
1344+
MDTWrapper->getDomTree().recordSplitCriticalEdge(this, Succ, NMBB);
13551345

13561346
auto *MLIWrapper = P.getAnalysisIfAvailable<MachineLoopInfoWrapperPass>();
13571347
if (MachineLoopInfo *MLI = MLIWrapper ? &MLIWrapper->getLI() : nullptr)

llvm/lib/CodeGen/MachineDominanceFrontier.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ char &llvm::MachineDominanceFrontierID = MachineDominanceFrontier::ID;
3838

3939
bool MachineDominanceFrontier::runOnMachineFunction(MachineFunction &) {
4040
releaseMemory();
41-
Base.analyze(getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree());
41+
Base.analyze(
42+
getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree().getBase());
4243
return false;
4344
}
4445

llvm/lib/CodeGen/MachineDominators.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ MachineDominatorTreeWrapperPass::MachineDominatorTreeWrapperPass()
9595
*PassRegistry::getPassRegistry());
9696
}
9797

98+
void MachineDominatorTree::calculate(MachineFunction &F) {
99+
CriticalEdgesToSplit.clear();
100+
NewBBs.clear();
101+
recalculate(F);
102+
}
103+
98104
char &llvm::MachineDominatorsID = MachineDominatorTreeWrapperPass::ID;
99105

100106
bool MachineDominatorTreeWrapperPass::runOnMachineFunction(MachineFunction &F) {
@@ -115,3 +121,71 @@ void MachineDominatorTreeWrapperPass::print(raw_ostream &OS,
115121
if (DT)
116122
DT->print(OS);
117123
}
124+
125+
void MachineDominatorTree::applySplitCriticalEdges() const {
126+
// Bail out early if there is nothing to do.
127+
if (CriticalEdgesToSplit.empty())
128+
return;
129+
130+
// For each element in CriticalEdgesToSplit, remember whether or not element
131+
// is the new immediate domminator of its successor. The mapping is done by
132+
// index, i.e., the information for the ith element of CriticalEdgesToSplit is
133+
// the ith element of IsNewIDom.
134+
SmallBitVector IsNewIDom(CriticalEdgesToSplit.size(), true);
135+
size_t Idx = 0;
136+
137+
// Collect all the dominance properties info, before invalidating
138+
// the underlying DT.
139+
for (CriticalEdge &Edge : CriticalEdgesToSplit) {
140+
// Update dominator information.
141+
MachineBasicBlock *Succ = Edge.ToBB;
142+
MachineDomTreeNode *SuccDTNode = Base::getNode(Succ);
143+
144+
for (MachineBasicBlock *PredBB : Succ->predecessors()) {
145+
if (PredBB == Edge.NewBB)
146+
continue;
147+
// If we are in this situation:
148+
// FromBB1 FromBB2
149+
// + +
150+
// + + + +
151+
// + + + +
152+
// ... Split1 Split2 ...
153+
// + +
154+
// + +
155+
// +
156+
// Succ
157+
// Instead of checking the domiance property with Split2, we check it with
158+
// FromBB2 since Split2 is still unknown of the underlying DT structure.
159+
if (NewBBs.count(PredBB)) {
160+
assert(PredBB->pred_size() == 1 && "A basic block resulting from a "
161+
"critical edge split has more "
162+
"than one predecessor!");
163+
PredBB = *PredBB->pred_begin();
164+
}
165+
if (!Base::dominates(SuccDTNode, Base::getNode(PredBB))) {
166+
IsNewIDom[Idx] = false;
167+
break;
168+
}
169+
}
170+
++Idx;
171+
}
172+
173+
// Now, update DT with the collected dominance properties info.
174+
Idx = 0;
175+
for (CriticalEdge &Edge : CriticalEdgesToSplit) {
176+
// We know FromBB dominates NewBB.
177+
MachineDomTreeNode *NewDTNode =
178+
const_cast<MachineDominatorTree *>(this)->Base::addNewBlock(
179+
Edge.NewBB, Edge.FromBB);
180+
181+
// If all the other predecessors of "Succ" are dominated by "Succ" itself
182+
// then the new block is the new immediate dominator of "Succ". Otherwise,
183+
// the new block doesn't dominate anything.
184+
if (IsNewIDom[Idx])
185+
const_cast<MachineDominatorTree *>(this)->Base::changeImmediateDominator(
186+
Base::getNode(Edge.ToBB), NewDTNode);
187+
++Idx;
188+
}
189+
NewBBs.clear();
190+
CriticalEdgesToSplit.clear();
191+
}

0 commit comments

Comments
 (0)