[CodeGen] Remove applySplitCriticalEdges in MachineDominatorTree
#97055
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-debuginfo Author: None (paperchalice) ChangesSummary:
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. Patch is 22.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97055.diff 13 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 01f3783a4ffb4..9f0f42a180774 100644
--- a/llvm/include/llvm/CodeGen/MachineDominators.h
+++ b/llvm/include/llvm/CodeGen/MachineDominators.h
@@ -73,86 +73,22 @@ 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) { calculate(MF); }
+ explicit MachineDominatorTree(MachineFunction &MF) { recalculate(MF); }
/// Handle invalidation explicitly.
bool invalidate(MachineFunction &, const PreservedAnalyses &PA,
MachineFunctionAnalysisManager::Invalidator &);
-
- // 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);
- }
+
+ using Base::dominates;
// 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);
@@ -164,107 +100,6 @@ 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.
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 2e787f4cd3db0..c34922b4170b0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1733,7 +1733,7 @@ void AsmPrinter::emitFunctionBody() {
MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
if (!MDT) {
OwnedMDT = std::make_unique<MachineDominatorTree>();
- OwnedMDT->getBase().recalculate(*MF);
+ OwnedMDT->recalculate(*MF);
MDT = OwnedMDT.get();
}
@@ -1741,7 +1741,7 @@ void AsmPrinter::emitFunctionBody() {
MLI = getAnalysisIfAvailable<MachineLoopInfo>();
if (!MLI) {
OwnedMLI = std::make_unique<MachineLoopInfo>();
- OwnedMLI->getBase().analyze(MDT->getBase());
+ OwnedMLI->getBase().analyze(*MDT);
MLI = OwnedMLI.get();
}
}
diff --git a/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp b/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
index 83b16fc883e8b..e2d3a69392df9 100644
--- a/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
+++ b/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
@@ -77,13 +77,13 @@ LazyMachineBlockFrequencyInfoPass::calculateIfNotAvailable() const {
if (!MDT) {
LLVM_DEBUG(dbgs() << "Building DominatorTree on the fly\n");
OwnedMDT = std::make_unique<MachineDominatorTree>();
- OwnedMDT->getBase().recalculate(*MF);
+ OwnedMDT->recalculate(*MF);
MDT = OwnedMDT.get();
}
// Generate LoopInfo from it.
OwnedMLI = std::make_unique<MachineLoopInfo>();
- OwnedMLI->getBase().analyze(MDT->getBase());
+ OwnedMLI->getBase().analyze(*MDT);
MLI = OwnedMLI.get();
}
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index 481d9e341da37..20df5cd8daaf2 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -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->getBase());
+ IDFCalculatorBase<MachineBasicBlock, false> IDF(*DomTree);
IDF.setLiveInBlocks(AllBlocks);
IDF.setDefiningBlocks(DefBlocks);
diff --git a/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp b/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
index 0c0a4e13c7c9e..15642969457aa 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
@@ -120,7 +120,7 @@ bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) {
MachineDominatorTree *DomTree = nullptr;
if (InstrRefBased) {
DomTree = &MDT;
- MDT.calculate(MF);
+ MDT.recalculate(MF);
TheImpl = &*InstrRefImpl;
}
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index 533ab7cccaeb7..4f0870fa533d1 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -1336,10 +1336,6 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
LIS->repairIntervalsInRange(this, getFirstTerminator(), end(), UsedRegs);
}
- if (auto *MDTWrapper =
- P.getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>())
- MDTWrapper->getDomTree().recordSplitCriticalEdge(this, Succ, NMBB);
-
if (MachineLoopInfo *MLI = P.getAnalysisIfAvailable<MachineLoopInfo>())
if (MachineLoop *TIL = MLI->getLoopFor(this)) {
// If one or the other blocks were not in a loop, the new block is not
diff --git a/llvm/lib/CodeGen/MachineDominanceFrontier.cpp b/llvm/lib/CodeGen/MachineDominanceFrontier.cpp
index 6a8ede4feb937..e3f312063d1ae 100644
--- a/llvm/lib/CodeGen/MachineDominanceFrontier.cpp
+++ b/llvm/lib/CodeGen/MachineDominanceFrontier.cpp
@@ -39,7 +39,7 @@ char &llvm::MachineDominanceFrontierID = MachineDominanceFrontier::ID;
bool MachineDominanceFrontier::runOnMachineFunction(MachineFunction &) {
releaseMemory();
Base.analyze(
- getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree().getBase());
+ getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree());
return false;
}
diff --git a/llvm/lib/CodeGen/MachineDominators.cpp b/llvm/lib/CodeGen/MachineDominators.cpp
index a2cc8fdfa7c9f..67a91c87bb1bc 100644
--- a/llvm/lib/CodeGen/MachineDominators.cpp
+++ b/llvm/lib/CodeGen/MachineDominators.cpp
@@ -95,12 +95,6 @@ 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) {
@@ -121,71 +115,3 @@ 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();
-}
diff --git a/llvm/lib/CodeGen/MachineLoopInfo.cpp b/llvm/lib/CodeGen/MachineLoopInfo.cpp
index 9fb103945838a..3b198b0711517 100644
--- a/llvm/lib/CodeGen/MachineLoopInfo.cpp
+++ b/llvm/lib/CodeGen/MachineLoopInfo.cpp
@@ -49,7 +49,7 @@ bool MachineLoopInfo::runOnMachineFunction(MachineFunction &) {
void MachineLoopInfo::calculate(MachineDominatorTree &MDT) {
releaseMemory();
- LI.analyze(MDT.getBase());
+ LI.analyze(MDT);
}
void MachineLoopInfo::getAnalysisUsage(AnalysisUsage &AU) const {
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 4dabaabe3659f..7f87050a2b14b 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -30,6 +30,7 @@
#include "llvm/CodeGen/MachineBlockFrequencyInfo.h"
#include "llvm/CodeGen/MachineBranchProbabilityInfo.h"
#include "llvm/CodeGen/MachineCycleAnalysis.h"
+#include "llvm/CodeGen/MachineDomTreeUpdater.h"
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -717,6 +718,8 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
RegClassInfo.runOnMachineFunction(MF);
TargetPassConfig *PassConfig = &getAnalysis<TargetPassConfig>();
EnableSinkAndFold = PassConfig->getEnableSinkAndFold();
+ MachineDomTreeUpdater MDTU(DT, PDT,
+ MachineDomTreeUpdater::UpdateStrategy::Lazy);
bool EverMadeChange = false;
@@ -743,6 +746,11 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
MadeChange = true;
++NumSplit;
CI->splitCriticalEdge(Pair.first, Pair.second, NewSucc);
+
+ MDTU.applyUpdates(
+ {{MachineDominatorTree::Insert, Pair.first, NewSucc},
+ {MachineDominatorTree::Insert, NewSucc, Pair.second},
+ {MachineDominatorTree::Delete, Pair.first, Pair.second}});
} else
LLVM_DEBUG(dbgs() << " *** Not legal to break critical edge\n");
}
diff --git a/llvm/lib/CodeGen/MachineUniformityAnalysis.cpp b/llvm/lib/CodeGen/MachineUniformityAnalysis.cpp
index 7548fc8141ec5..35eaa7390713e 100644
--- a/llvm/lib/CodeGen/MachineUniformityAnalysis.cpp
+++ b/llvm/lib/CodeGen/MachineUniformityAnalysis.cpp
@@ -200,7 +200,7 @@ void MachineUniformityAnalysisPass::getAnalysisUsage(AnalysisUsage &AU) const {
bool MachineUniformityAnalysisPass::runOnMachineFunction(MachineFunction &MF) {
auto &DomTree =
- getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree().getBase();
+ getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
auto &CI = getAnalysis<MachineCycleInfoWrapperPass>().getCycleInfo();
// FIXME: Query TTI::hasBranchDivergence. -run-pass seems to end up with a
// default NoTTI
diff --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 592972f5c83b2..69d10cd9bf944 100644
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -21,6 +21,7 @@
#include "llvm/CodeGen/LiveIntervals.h"
#include "llvm/CodeGen/LiveVariables.h"
#include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineDomTreeUpdater.h"
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -95,7 +96,8 @@ namespace {
/// Split critical edges where necessary for good coalescer performance.
bool SplitPHIEdges(MachineFunction &MF, MachineBasicBlock &MBB,
MachineLoopInfo *MLI,
- std::vector<SparseBitVector<>> *LiveInSets);
+ std::vector<SparseBitVector<>> *LiveInSets,
+ MachineDomTreeUpdater *MDTU);
// These functions are temporary abstractions around LiveVariables and
// LiveIntervals, so they can go away when LiveVariables does.
@@ -183,8 +185,13 @@ bool PHIElimination::runOnMachineFunction(MachineFunction &MF) {
}
MachineLoopInfo *MLI = getAnalysisIfAvailable<MachineLoopInfo>();
+ auto *DTWrapper = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
+ MachineDominatorTree *MDT = DTWrapper ? &DTWrapper->getDomTree() : nullptr;
+ MachineDomTreeUpdater MDTU(MDT,
+ MachineDomTreeUpdater::UpdateStrategy::Lazy);
for (auto &MBB : MF)
- Changed |= SplitPHIEdges(MF, MBB, MLI, (LV ? &LiveInSets : nullptr));
+ Changed |= SplitPHIEdges(MF, MBB, MLI, (LV ? &LiveInSets : nullptr),
+ DTWrapper ? &MDTU : nullptr);
}
// This pass takes the function out of SSA form.
@@ -217,7 +224,7 @@ bool PHIElimination::runOnMachineFunction(MachineFunction &MF) {
// TODO: we should use the incremental DomTree updater here.
if (Changed)
if (auto *MDT = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>())
- MDT->getDomTree().getBase().recalculate(MF);
+ MDT->getDomTree().recalculate(M...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
paperchalice
force-pushed
the
mdt
branch
3 times, most recently
from
25399ca to
435def0
Compare
21 tasks
paperchalice
force-pushed
the
mdt
branch
2 times, most recently
from
45b51e7 to
08b39ed
Compare
TEST 'BOLT :: X86/reader-stale-yaml-std.test' FAILEDAlso seen in other PRs. |
paperchalice
force-pushed
the
mdt
branch
2 times, most recently
from
1304648 to
9c02089
Compare
paperchalice
force-pushed
the
mdt
branch
2 times, most recently
from
db9fd57 to
befc70e
Compare
Could we instead have the MachineBasicBlock use the Having each user of "split critical edge" replicate the call to MDTU doesn't sound like a great trade-off to me. Other than that, +1 on removing the custom update logic and using the more standard one. |
|
Move to New failiures: Tested locally, |
paperchalice
force-pushed
the
mdt
branch
2 times, most recently
from
a8a86c8 to
e953eaa
Compare
paperchalice
commented
Jul 3, 2024
| FirstInLoop = true; | ||
| HoistOutOfLoop(N, CurLoop, CurPreheader); |
There was a problem hiding this comment.
N is no longer a node in DT when we split critical edges eagerly in failed tests before.
|
ping @qcolombet |
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.
qcolombet
approved these changes
Jul 10, 2024
|
Reverted due to large compile-time regressions, see http://llvm-compile-time-tracker.com/compare.php?from=cb3bc5be9c20d893adf94cdf436092657ab5ab40&to=c5e5088033fed170068d818c54af6862e449b545&stat=instructions:u. |
nikic
reviewed
Jul 11, 2024
| MachineDomTreeUpdater::UpdateStrategy::Eager); | ||
| MDTU.applyUpdates({{MachineDominatorTree::Insert, this, NMBB}, | ||
| {MachineDominatorTree::Insert, NMBB, Succ}, | ||
| {MachineDominatorTree::Delete, this, Succ}}); |
There was a problem hiding this comment.
This means that you are now doing DT updates for critical edge splitting one by one, instead of batching many together.
paperchalice
added a commit
to paperchalice/llvm-project
that referenced
this pull request
Jul 11, 2024
…torTree` (llvm#97055)" This reverts commit 6a90769.
paperchalice
added a commit
to paperchalice/llvm-project
that referenced
this pull request
Jul 11, 2024
…torTree` (llvm#97055)" This reverts commit 6a90769.
paperchalice
added a commit
to paperchalice/llvm-project
that referenced
this pull request
Jul 11, 2024
…torTree` (llvm#97055)" This reverts commit 6a90769.
paperchalice
added a commit
to paperchalice/llvm-project
that referenced
this pull request
Jul 11, 2024
…torTree` (llvm#97055)" This reverts commit 6a90769.
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
…lvm#97055) 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.
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
…orTree` (llvm#97055)" This reverts commit c5e5088. Causes large compile-time regressions.
paperchalice
added a commit
to paperchalice/llvm-project
that referenced
this pull request
Jul 23, 2024
…torTree` (llvm#97055)" This reverts commit 6a90769.
nikic
pushed a commit
to nikic/llvm-project
that referenced
this pull request
Jul 23, 2024
…torTree` (llvm#97055)" This reverts commit 6a90769.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
MachineDominatorTree.MachineDominatorTreeupdate code inMachineBasicBlock::SplitCriticalEdge.MachineDomTreeUpdaterin passes which callMachineBasicBlock::SplitCriticalEdgeand preserveMachineDominatorTreeWrapperPassor 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.