Skip to content

Commit

Permalink
Revert "[CodeGen] Remove applySplitCriticalEdges in `MachineDominat…
Browse files Browse the repository at this point in the history
…orTree` (llvm#97055)"

This reverts commit c5e5088.

Causes large compile-time regressions.
  • Loading branch information
nikic committed Jul 11, 2024
1 parent 7868033 commit 6a90769
Show file tree
Hide file tree
Showing 21 changed files with 284 additions and 44 deletions.
169 changes: 167 additions & 2 deletions llvm/include/llvm/CodeGen/MachineDominators.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,86 @@ extern template bool Verify<MBBDomTree>(const MBBDomTree &DT,
/// compute a normal dominator tree.
///
class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
/// Helper structure used to hold all the basic blocks
/// involved in the split of a critical edge.
struct CriticalEdge {
MachineBasicBlock *FromBB;
MachineBasicBlock *ToBB;
MachineBasicBlock *NewBB;
};

/// Pile up all the critical edges to be split.
/// The splitting of a critical edge is local and thus, it is possible
/// to apply several of those changes at the same time.
mutable SmallVector<CriticalEdge, 32> CriticalEdgesToSplit;

/// Remember all the basic blocks that are inserted during
/// edge splitting.
/// Invariant: NewBBs == all the basic blocks contained in the NewBB
/// field of all the elements of CriticalEdgesToSplit.
/// I.e., forall elt in CriticalEdgesToSplit, it exists BB in NewBBs
/// such as BB == elt.NewBB.
mutable SmallSet<MachineBasicBlock *, 32> NewBBs;

/// Apply all the recorded critical edges to the DT.
/// This updates the underlying DT information in a way that uses
/// the fast query path of DT as much as possible.
/// FIXME: This method should not be a const member!
///
/// \post CriticalEdgesToSplit.empty().
void applySplitCriticalEdges() const;

public:
using Base = DomTreeBase<MachineBasicBlock>;

MachineDominatorTree() = default;
explicit MachineDominatorTree(MachineFunction &MF) { recalculate(MF); }
explicit MachineDominatorTree(MachineFunction &MF) { calculate(MF); }

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

using Base::dominates;
// FIXME: If there is an updater for MachineDominatorTree,
// migrate to this updater and remove these wrappers.

MachineDominatorTree &getBase() {
applySplitCriticalEdges();
return *this;
}

MachineBasicBlock *getRoot() const {
applySplitCriticalEdges();
return Base::getRoot();
}

MachineDomTreeNode *getRootNode() const {
applySplitCriticalEdges();
return const_cast<MachineDomTreeNode *>(Base::getRootNode());
}

void calculate(MachineFunction &F);

bool dominates(const MachineDomTreeNode *A,
const MachineDomTreeNode *B) const {
applySplitCriticalEdges();
return Base::dominates(A, B);
}

void getDescendants(MachineBasicBlock *A,
SmallVectorImpl<MachineBasicBlock *> &Result) {
applySplitCriticalEdges();
Base::getDescendants(A, Result);
}

bool dominates(const MachineBasicBlock *A, const MachineBasicBlock *B) const {
applySplitCriticalEdges();
return Base::dominates(A, B);
}

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

return &*I == A;
}

bool properlyDominates(const MachineDomTreeNode *A,
const MachineDomTreeNode *B) const {
applySplitCriticalEdges();
return Base::properlyDominates(A, B);
}

bool properlyDominates(const MachineBasicBlock *A,
const MachineBasicBlock *B) const {
applySplitCriticalEdges();
return Base::properlyDominates(A, B);
}

/// findNearestCommonDominator - Find nearest common dominator basic block
/// for basic block A and B. If there is no such block then return NULL.
MachineBasicBlock *findNearestCommonDominator(MachineBasicBlock *A,
MachineBasicBlock *B) {
applySplitCriticalEdges();
return Base::findNearestCommonDominator(A, B);
}

MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
applySplitCriticalEdges();
return Base::getNode(BB);
}

/// getNode - return the (Post)DominatorTree node for the specified basic
/// block. This is the same as using operator[] on this class.
///
MachineDomTreeNode *getNode(MachineBasicBlock *BB) const {
applySplitCriticalEdges();
return Base::getNode(BB);
}

/// addNewBlock - Add a new node to the dominator tree information. This
/// creates a new node as a child of DomBB dominator node,linking it into
/// the children list of the immediate dominator.
MachineDomTreeNode *addNewBlock(MachineBasicBlock *BB,
MachineBasicBlock *DomBB) {
applySplitCriticalEdges();
return Base::addNewBlock(BB, DomBB);
}

/// changeImmediateDominator - This method is used to update the dominator
/// tree information when a node's immediate dominator changes.
///
void changeImmediateDominator(MachineBasicBlock *N,
MachineBasicBlock *NewIDom) {
applySplitCriticalEdges();
Base::changeImmediateDominator(N, NewIDom);
}

void changeImmediateDominator(MachineDomTreeNode *N,
MachineDomTreeNode *NewIDom) {
applySplitCriticalEdges();
Base::changeImmediateDominator(N, NewIDom);
}

/// eraseNode - Removes a node from the dominator tree. Block must not
/// dominate any other blocks. Removes node from its immediate dominator's
/// children list. Deletes dominator node associated with basic block BB.
void eraseNode(MachineBasicBlock *BB) {
applySplitCriticalEdges();
Base::eraseNode(BB);
}

/// splitBlock - BB is split and now it has one successor. Update dominator
/// tree to reflect this change.
void splitBlock(MachineBasicBlock* NewBB) {
applySplitCriticalEdges();
Base::splitBlock(NewBB);
}

/// isReachableFromEntry - Return true if A is dominated by the entry
/// block of the function containing it.
bool isReachableFromEntry(const MachineBasicBlock *A) {
applySplitCriticalEdges();
return Base::isReachableFromEntry(A);
}

/// Record that the critical edge (FromBB, ToBB) has been
/// split with NewBB.
/// This is best to use this method instead of directly update the
/// underlying information, because this helps mitigating the
/// number of time the DT information is invalidated.
///
/// \note Do not use this method with regular edges.
///
/// \note To benefit from the compile time improvement incurred by this
/// method, the users of this method have to limit the queries to the DT
/// interface between two edges splitting. In other words, they have to
/// pack the splitting of critical edges as much as possible.
void recordSplitCriticalEdge(MachineBasicBlock *FromBB,
MachineBasicBlock *ToBB,
MachineBasicBlock *NewBB) {
bool Inserted = NewBBs.insert(NewBB).second;
(void)Inserted;
assert(Inserted &&
"A basic block inserted via edge splitting cannot appear twice");
CriticalEdgesToSplit.push_back({FromBB, ToBB, NewBB});
}
};

/// \brief Analysis pass which computes a \c MachineDominatorTree.
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1698,7 +1698,7 @@ void AsmPrinter::emitFunctionBody() {
MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
if (!MDT) {
OwnedMDT = std::make_unique<MachineDominatorTree>();
OwnedMDT->recalculate(*MF);
OwnedMDT->getBase().recalculate(*MF);
MDT = OwnedMDT.get();
}

Expand All @@ -1707,7 +1707,7 @@ void AsmPrinter::emitFunctionBody() {
MLI = MLIWrapper ? &MLIWrapper->getLI() : nullptr;
if (!MLI) {
OwnedMLI = std::make_unique<MachineLoopInfo>();
OwnedMLI->analyze(*MDT);
OwnedMLI->analyze(MDT->getBase());
MLI = OwnedMLI.get();
}
}
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ LazyMachineBlockFrequencyInfoPass::calculateIfNotAvailable() const {
if (!MDT) {
LLVM_DEBUG(dbgs() << "Building DominatorTree on the fly\n");
OwnedMDT = std::make_unique<MachineDominatorTree>();
OwnedMDT->recalculate(*MF);
OwnedMDT->getBase().recalculate(*MF);
MDT = OwnedMDT.get();
}

// Generate LoopInfo from it.
OwnedMLI = std::make_unique<MachineLoopInfo>();
OwnedMLI->analyze(*MDT);
OwnedMLI->analyze(MDT->getBase());
MLI = OwnedMLI.get();
}

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2754,7 +2754,7 @@ void InstrRefBasedLDV::BlockPHIPlacement(
// Apply IDF calculator to the designated set of location defs, storing
// required PHIs into PHIBlocks. Uses the dominator tree stored in the
// InstrRefBasedLDV object.
IDFCalculatorBase<MachineBasicBlock, false> IDF(*DomTree);
IDFCalculatorBase<MachineBasicBlock, false> IDF(DomTree->getBase());

IDF.setLiveInBlocks(AllBlocks);
IDF.setDefiningBlocks(DefBlocks);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) {
MachineDominatorTree *DomTree = nullptr;
if (InstrRefBased) {
DomTree = &MDT;
MDT.recalculate(MF);
MDT.calculate(MF);
TheImpl = &*InstrRefImpl;
}

Expand Down
16 changes: 3 additions & 13 deletions llvm/lib/CodeGen/MachineBasicBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@
#include "llvm/CodeGen/LiveIntervals.h"
#include "llvm/CodeGen/LivePhysRegs.h"
#include "llvm/CodeGen/LiveVariables.h"
#include "llvm/CodeGen/MachineDomTreeUpdater.h"
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineJumpTableInfo.h"
#include "llvm/CodeGen/MachineLoopInfo.h"
#include "llvm/CodeGen/MachinePostDominators.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/SlotIndexes.h"
#include "llvm/CodeGen/TargetInstrInfo.h"
Expand Down Expand Up @@ -1341,17 +1339,9 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
LIS->repairIntervalsInRange(this, getFirstTerminator(), end(), UsedRegs);
}

auto *MDTWrapper =
P.getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
auto *MPDTWrapper =
P.getAnalysisIfAvailable<MachinePostDominatorTreeWrapperPass>();
auto *MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
auto *MPDT = MPDTWrapper ? &MPDTWrapper->getPostDomTree() : nullptr;
MachineDomTreeUpdater MDTU(MDT, MPDT,
MachineDomTreeUpdater::UpdateStrategy::Eager);
MDTU.applyUpdates({{MachineDominatorTree::Insert, this, NMBB},
{MachineDominatorTree::Insert, NMBB, Succ},
{MachineDominatorTree::Delete, this, Succ}});
if (auto *MDTWrapper =
P.getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>())
MDTWrapper->getDomTree().recordSplitCriticalEdge(this, Succ, NMBB);

auto *MLIWrapper = P.getAnalysisIfAvailable<MachineLoopInfoWrapperPass>();
if (MachineLoopInfo *MLI = MLIWrapper ? &MLIWrapper->getLI() : nullptr)
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/MachineDominanceFrontier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ char &llvm::MachineDominanceFrontierID = MachineDominanceFrontier::ID;

bool MachineDominanceFrontier::runOnMachineFunction(MachineFunction &) {
releaseMemory();
Base.analyze(getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree());
Base.analyze(
getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree().getBase());
return false;
}

Expand Down
74 changes: 74 additions & 0 deletions llvm/lib/CodeGen/MachineDominators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ MachineDominatorTreeWrapperPass::MachineDominatorTreeWrapperPass()
*PassRegistry::getPassRegistry());
}

void MachineDominatorTree::calculate(MachineFunction &F) {
CriticalEdgesToSplit.clear();
NewBBs.clear();
recalculate(F);
}

char &llvm::MachineDominatorsID = MachineDominatorTreeWrapperPass::ID;

bool MachineDominatorTreeWrapperPass::runOnMachineFunction(MachineFunction &F) {
Expand All @@ -115,3 +121,71 @@ void MachineDominatorTreeWrapperPass::print(raw_ostream &OS,
if (DT)
DT->print(OS);
}

void MachineDominatorTree::applySplitCriticalEdges() const {
// Bail out early if there is nothing to do.
if (CriticalEdgesToSplit.empty())
return;

// For each element in CriticalEdgesToSplit, remember whether or not element
// is the new immediate domminator of its successor. The mapping is done by
// index, i.e., the information for the ith element of CriticalEdgesToSplit is
// the ith element of IsNewIDom.
SmallBitVector IsNewIDom(CriticalEdgesToSplit.size(), true);
size_t Idx = 0;

// Collect all the dominance properties info, before invalidating
// the underlying DT.
for (CriticalEdge &Edge : CriticalEdgesToSplit) {
// Update dominator information.
MachineBasicBlock *Succ = Edge.ToBB;
MachineDomTreeNode *SuccDTNode = Base::getNode(Succ);

for (MachineBasicBlock *PredBB : Succ->predecessors()) {
if (PredBB == Edge.NewBB)
continue;
// If we are in this situation:
// FromBB1 FromBB2
// + +
// + + + +
// + + + +
// ... Split1 Split2 ...
// + +
// + +
// +
// Succ
// Instead of checking the domiance property with Split2, we check it with
// FromBB2 since Split2 is still unknown of the underlying DT structure.
if (NewBBs.count(PredBB)) {
assert(PredBB->pred_size() == 1 && "A basic block resulting from a "
"critical edge split has more "
"than one predecessor!");
PredBB = *PredBB->pred_begin();
}
if (!Base::dominates(SuccDTNode, Base::getNode(PredBB))) {
IsNewIDom[Idx] = false;
break;
}
}
++Idx;
}

// Now, update DT with the collected dominance properties info.
Idx = 0;
for (CriticalEdge &Edge : CriticalEdgesToSplit) {
// We know FromBB dominates NewBB.
MachineDomTreeNode *NewDTNode =
const_cast<MachineDominatorTree *>(this)->Base::addNewBlock(
Edge.NewBB, Edge.FromBB);

// If all the other predecessors of "Succ" are dominated by "Succ" itself
// then the new block is the new immediate dominator of "Succ". Otherwise,
// the new block doesn't dominate anything.
if (IsNewIDom[Idx])
const_cast<MachineDominatorTree *>(this)->Base::changeImmediateDominator(
Base::getNode(Edge.ToBB), NewDTNode);
++Idx;
}
NewBBs.clear();
CriticalEdgesToSplit.clear();
}
Loading

0 comments on commit 6a90769

Please sign in to comment.