Skip to content

Commit befc70e

Browse files
paperchalicepaperchalice
committed
[CodeGen] Remove applySplitCriticalEdges in MachineDominatorTree
Summary: - Remove wrappers in `MachineDominatorTree`. - Remove `MachineDominatorTree` update code in `MachineBasicBlock::SplitCriticalEdge` - Use `MachineDomTreeUpdater` in passes which call `MachineBasicBlock::SplitCriticalEdge` and preserve `MachineDominatorTreeWrapperPass` or CFG analyses. Commit abea99f introduced related methods in 2014. Now we have SemiNCA based dominator tree in 2017 and dominator tree updater, the solution adopted here seems a bit outdated.
1 parent 001da22 commit befc70e

20 files changed

+54
-283
lines changed

‎llvm/include/llvm/CodeGen/MachineDominators.h

Lines changed: 2 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -73,86 +73,22 @@ 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;
10476

10577
public:
10678
using Base = DomTreeBase<MachineBasicBlock>;
10779

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

11183
/// Handle invalidation explicitly.
11284
bool invalidate(MachineFunction &, const PreservedAnalyses &PA,
11385
MachineFunctionAnalysisManager::Invalidator &);
11486

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-
}
87+
using Base::dominates;
15188

15289
// dominates - Return true if A dominates B. This performs the
15390
// special checks necessary if A and B are in the same basic block.
15491
bool dominates(const MachineInstr *A, const MachineInstr *B) const {
155-
applySplitCriticalEdges();
15692
const MachineBasicBlock *BBA = A->getParent(), *BBB = B->getParent();
15793
if (BBA != BBB)
15894
return Base::dominates(BBA, BBB);
@@ -164,107 +100,6 @@ class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
164100

165101
return &*I == A;
166102
}
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-
}
268103
};
269104

270105
/// \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
@@ -1735,15 +1735,15 @@ void AsmPrinter::emitFunctionBody() {
17351735
MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
17361736
if (!MDT) {
17371737
OwnedMDT = std::make_unique<MachineDominatorTree>();
1738-
OwnedMDT->getBase().recalculate(*MF);
1738+
OwnedMDT->recalculate(*MF);
17391739
MDT = OwnedMDT.get();
17401740
}
17411741

17421742
// Get MachineLoopInfo or compute it on the fly if it's unavailable
17431743
MLI = getAnalysisIfAvailable<MachineLoopInfo>();
17441744
if (!MLI) {
17451745
OwnedMLI = std::make_unique<MachineLoopInfo>();
1746-
OwnedMLI->getBase().analyze(MDT->getBase());
1746+
OwnedMLI->getBase().analyze(*MDT);
17471747
MLI = OwnedMLI.get();
17481748
}
17491749
}

‎llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp

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

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

‎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->getBase());
2757+
IDFCalculatorBase<MachineBasicBlock, false> IDF(*DomTree);
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.calculate(MF);
123+
MDT.recalculate(MF);
124124
TheImpl = &*InstrRefImpl;
125125
}
126126

‎llvm/lib/CodeGen/MachineBasicBlock.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,10 +1336,6 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
13361336
LIS->repairIntervalsInRange(this, getFirstTerminator(), end(), UsedRegs);
13371337
}
13381338

1339-
if (auto *MDTWrapper =
1340-
P.getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>())
1341-
MDTWrapper->getDomTree().recordSplitCriticalEdge(this, Succ, NMBB);
1342-
13431339
if (MachineLoopInfo *MLI = P.getAnalysisIfAvailable<MachineLoopInfo>())
13441340
if (MachineLoop *TIL = MLI->getLoopFor(this)) {
13451341
// If one or the other blocks were not in a loop, the new block is not

‎llvm/lib/CodeGen/MachineDominanceFrontier.cpp

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

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

‎llvm/lib/CodeGen/MachineDominators.cpp

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

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

106100
bool MachineDominatorTreeWrapperPass::runOnMachineFunction(MachineFunction &F) {
@@ -121,71 +115,3 @@ void MachineDominatorTreeWrapperPass::print(raw_ostream &OS,
121115
if (DT)
122116
DT->print(OS);
123117
}
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-
}

‎llvm/lib/CodeGen/MachineLoopInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ bool MachineLoopInfo::runOnMachineFunction(MachineFunction &) {
4949

5050
void MachineLoopInfo::calculate(MachineDominatorTree &MDT) {
5151
releaseMemory();
52-
LI.analyze(MDT.getBase());
52+
LI.analyze(MDT);
5353
}
5454

5555
void MachineLoopInfo::getAnalysisUsage(AnalysisUsage &AU) const {

‎llvm/lib/CodeGen/MachineSink.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "llvm/CodeGen/MachineBlockFrequencyInfo.h"
3131
#include "llvm/CodeGen/MachineBranchProbabilityInfo.h"
3232
#include "llvm/CodeGen/MachineCycleAnalysis.h"
33+
#include "llvm/CodeGen/MachineDomTreeUpdater.h"
3334
#include "llvm/CodeGen/MachineDominators.h"
3435
#include "llvm/CodeGen/MachineFunction.h"
3536
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -721,6 +722,10 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
721722
bool EverMadeChange = false;
722723

723724
while (true) {
725+
// Ensure that the dominant tree is up-to-date after splitting the critical
726+
// edge.
727+
MachineDomTreeUpdater MDTU(DT, PDT,
728+
MachineDomTreeUpdater::UpdateStrategy::Lazy);
724729
bool MadeChange = false;
725730

726731
// Process all basic blocks.
@@ -743,6 +748,11 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
743748
MadeChange = true;
744749
++NumSplit;
745750
CI->splitCriticalEdge(Pair.first, Pair.second, NewSucc);
751+
752+
MDTU.applyUpdates(
753+
{{MachineDominatorTree::Insert, Pair.first, NewSucc},
754+
{MachineDominatorTree::Insert, NewSucc, Pair.second},
755+
{MachineDominatorTree::Delete, Pair.first, Pair.second}});
746756
} else
747757
LLVM_DEBUG(dbgs() << " *** Not legal to break critical edge\n");
748758
}

0 commit comments

Comments
 (0)