[llvm] [CodeGen] Remove `applySplitCriticalEdges` in `MachineDominatorTree` (PR #97055)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 01:59:06 PDT 2024


https://github.com/paperchalice updated https://github.com/llvm/llvm-project/pull/97055

>From 9d871ba6795f08dd0413a9b4a521831ea3e2d155 Mon Sep 17 00:00:00 2001
From: PaperChalice <liujunchang97 at outlook.com>
Date: Fri, 28 Jun 2024 16:14:27 +0800
Subject: [PATCH 1/2] [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 abea99f65a97248974c02a5544eaf25fc4240056 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.
---
 llvm/include/llvm/CodeGen/MachineDominators.h | 169 +-----------------
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp    |   4 +-
 .../CodeGen/LazyMachineBlockFrequencyInfo.cpp |   4 +-
 .../LiveDebugValues/InstrRefBasedImpl.cpp     |   2 +-
 .../LiveDebugValues/LiveDebugValues.cpp       |   2 +-
 llvm/lib/CodeGen/MachineBasicBlock.cpp        |   4 -
 llvm/lib/CodeGen/MachineDominanceFrontier.cpp |   3 +-
 llvm/lib/CodeGen/MachineDominators.cpp        |  74 --------
 llvm/lib/CodeGen/MachineLoopInfo.cpp          |   2 +-
 llvm/lib/CodeGen/MachineSink.cpp              |  10 ++
 .../lib/CodeGen/MachineUniformityAnalysis.cpp |   3 +-
 llvm/lib/CodeGen/PHIElimination.cpp           |  29 +--
 llvm/lib/CodeGen/XRayInstrumentation.cpp      |   4 +-
 .../lib/Target/AMDGPU/AMDGPURegBankSelect.cpp |   5 +-
 .../Target/AMDGPU/SILateBranchLowering.cpp    |   6 +-
 llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp    |   2 +-
 llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp    |   2 +-
 .../Target/Hexagon/HexagonFrameLowering.cpp   |   2 +-
 .../deltas/ReduceInstructionsMIR.cpp          |   2 +-
 .../WebAssemblyExceptionInfoTest.cpp          |   8 +-
 20 files changed, 54 insertions(+), 283 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 74cf94398736d..61635ff64502d 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 c52cbff689dc5..5e41562bbbbf6 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1699,7 +1699,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();
     }
 
@@ -1707,7 +1707,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 555cbb7a507f4..98099ace804dd 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..ed69ed931c5cb 100644
--- a/llvm/lib/CodeGen/MachineDominanceFrontier.cpp
+++ b/llvm/lib/CodeGen/MachineDominanceFrontier.cpp
@@ -38,8 +38,7 @@ char &llvm::MachineDominanceFrontierID = MachineDominanceFrontier::ID;
 
 bool MachineDominanceFrontier::runOnMachineFunction(MachineFunction &) {
   releaseMemory();
-  Base.analyze(
-      getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree().getBase());
+  Base.analyze(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..7073a90c09652 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"
@@ -721,6 +722,10 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
   bool EverMadeChange = false;
 
   while (true) {
+    // Ensure that the dominant tree is up-to-date after splitting the critical
+    // edge.
+    MachineDomTreeUpdater MDTU(DT, PDT,
+                               MachineDomTreeUpdater::UpdateStrategy::Lazy);
     bool MadeChange = false;
 
     // Process all basic blocks.
@@ -743,6 +748,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..a4b78c1c75ceb 100644
--- a/llvm/lib/CodeGen/MachineUniformityAnalysis.cpp
+++ b/llvm/lib/CodeGen/MachineUniformityAnalysis.cpp
@@ -199,8 +199,7 @@ void MachineUniformityAnalysisPass::getAnalysisUsage(AnalysisUsage &AU) const {
 }
 
 bool MachineUniformityAnalysisPass::runOnMachineFunction(MachineFunction &MF) {
-  auto &DomTree =
-      getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree().getBase();
+  auto &DomTree = 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..74c3102f9982b 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.
@@ -214,11 +221,6 @@ bool PHIElimination::runOnMachineFunction(MachineFunction &MF) {
     MF.deleteMachineInstr(I.first);
   }
 
-  // TODO: we should use the incremental DomTree updater here.
-  if (Changed)
-    if (auto *MDT = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>())
-      MDT->getDomTree().getBase().recalculate(MF);
-
   LoweredPHIs.clear();
   ImpDefs.clear();
   VRegPHIUseCount.clear();
@@ -673,10 +675,10 @@ void PHIElimination::analyzePHINodes(const MachineFunction& MF) {
   }
 }
 
-bool PHIElimination::SplitPHIEdges(MachineFunction &MF,
-                                   MachineBasicBlock &MBB,
+bool PHIElimination::SplitPHIEdges(MachineFunction &MF, MachineBasicBlock &MBB,
                                    MachineLoopInfo *MLI,
-                                   std::vector<SparseBitVector<>> *LiveInSets) {
+                                   std::vector<SparseBitVector<>> *LiveInSets,
+                                   MachineDomTreeUpdater *MDTU) {
   if (MBB.empty() || !MBB.front().isPHI() || MBB.isEHPad())
     return false;   // Quick exit for basic blocks without PHIs.
 
@@ -743,7 +745,12 @@ bool PHIElimination::SplitPHIEdges(MachineFunction &MF,
       }
       if (!ShouldSplit && !SplitAllCriticalEdges)
         continue;
-      if (!PreMBB->SplitCriticalEdge(&MBB, *this, LiveInSets)) {
+      if (auto *NewMBB = PreMBB->SplitCriticalEdge(&MBB, *this, LiveInSets)) {
+        if (MDTU)
+          MDTU->applyUpdates({{MachineDominatorTree::Insert, PreMBB, NewMBB},
+                              {MachineDominatorTree::Insert, NewMBB, &MBB},
+                              {MachineDominatorTree::Delete, PreMBB, &MBB}});
+      } else {
         LLVM_DEBUG(dbgs() << "Failed to split critical edge.\n");
         continue;
       }
diff --git a/llvm/lib/CodeGen/XRayInstrumentation.cpp b/llvm/lib/CodeGen/XRayInstrumentation.cpp
index a74362e888397..2541aaa261791 100644
--- a/llvm/lib/CodeGen/XRayInstrumentation.cpp
+++ b/llvm/lib/CodeGen/XRayInstrumentation.cpp
@@ -175,7 +175,7 @@ bool XRayInstrumentation::runOnMachineFunction(MachineFunction &MF) {
       auto *MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
       MachineDominatorTree ComputedMDT;
       if (!MDT) {
-        ComputedMDT.getBase().recalculate(MF);
+        ComputedMDT.recalculate(MF);
         MDT = &ComputedMDT;
       }
 
@@ -183,7 +183,7 @@ bool XRayInstrumentation::runOnMachineFunction(MachineFunction &MF) {
       auto *MLI = getAnalysisIfAvailable<MachineLoopInfo>();
       MachineLoopInfo ComputedMLI;
       if (!MLI) {
-        ComputedMLI.getBase().analyze(MDT->getBase());
+        ComputedMLI.getBase().analyze(*MDT);
         MLI = &ComputedMLI;
       }
 
diff --git a/llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp b/llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp
index d1985f46b1c44..8135805cd696b 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp
@@ -66,9 +66,8 @@ bool AMDGPURegBankSelect::runOnMachineFunction(MachineFunction &MF) {
   MachineDominatorTree &DomTree =
       getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
 
-  MachineUniformityInfo Uniformity =
-      computeMachineUniformityInfo(MF, CycleInfo, DomTree.getBase(),
-                                   !ST.isSingleLaneExecution(F));
+  MachineUniformityInfo Uniformity = computeMachineUniformityInfo(
+      MF, CycleInfo, DomTree, !ST.isSingleLaneExecution(F));
   (void)Uniformity; // TODO: Use this
 
   assignRegisterBanks(MF);
diff --git a/llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp b/llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp
index afc6353ec8116..6d1ad90a83147 100644
--- a/llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp
@@ -114,7 +114,7 @@ static void splitBlock(MachineBasicBlock &MBB, MachineInstr &MI,
     DTUpdates.push_back({DomTreeT::Delete, &MBB, Succ});
   }
   DTUpdates.push_back({DomTreeT::Insert, &MBB, SplitBB});
-  MDT->getBase().applyUpdates(DTUpdates);
+  MDT->applyUpdates(DTUpdates);
 }
 
 void SILateBranchLowering::expandChainCall(MachineInstr &MI) {
@@ -142,7 +142,7 @@ void SILateBranchLowering::earlyTerm(MachineInstr &MI,
     splitBlock(MBB, *BranchMI, MDT);
 
   MBB.addSuccessor(EarlyExitBlock);
-  MDT->getBase().insertEdge(&MBB, EarlyExitBlock);
+  MDT->insertEdge(&MBB, EarlyExitBlock);
 }
 
 bool SILateBranchLowering::runOnMachineFunction(MachineFunction &MF) {
@@ -238,7 +238,7 @@ bool SILateBranchLowering::runOnMachineFunction(MachineFunction &MF) {
       }
 
       MBB->addSuccessor(EmptyMBBAtEnd);
-      MDT->getBase().insertEdge(MBB, EmptyMBBAtEnd);
+      MDT->insertEdge(MBB, EmptyMBBAtEnd);
       BuildMI(*MBB, MI, MI->getDebugLoc(), TII->get(AMDGPU::S_BRANCH))
           .addMBB(EmptyMBBAtEnd);
       MI->eraseFromParent();
diff --git a/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp b/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp
index a9ee74dec1203..056ef62cd82e8 100644
--- a/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp
@@ -548,7 +548,7 @@ bool PhiLoweringHelper::lowerPhis() {
   if (Vreg1Phis.empty())
     return false;
 
-  DT->getBase().updateDFSNumbers();
+  DT->updateDFSNumbers();
   MachineBasicBlock *PrevMBB = nullptr;
   for (MachineInstr *MI : Vreg1Phis) {
     MachineBasicBlock &MBB = *MI->getParent();
diff --git a/llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp b/llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp
index 742fd397ff9e3..05bcbbda3a213 100644
--- a/llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp
+++ b/llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp
@@ -781,7 +781,7 @@ MachineBasicBlock *SIWholeQuadMode::splitBlock(MachineBasicBlock *BB,
     }
     DTUpdates.push_back({DomTreeT::Insert, BB, SplitBB});
     if (MDT)
-      MDT->getBase().applyUpdates(DTUpdates);
+      MDT->applyUpdates(DTUpdates);
     if (PDT)
       PDT->applyUpdates(DTUpdates);
 
diff --git a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
index f4f84beea734d..dc8326a657bb2 100644
--- a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
@@ -413,7 +413,7 @@ void HexagonFrameLowering::findShrunkPrologEpilog(MachineFunction &MF,
   auto &HRI = *MF.getSubtarget<HexagonSubtarget>().getRegisterInfo();
 
   MachineDominatorTree MDT;
-  MDT.calculate(MF);
+  MDT.recalculate(MF);
   MachinePostDominatorTree MPT;
   MPT.recalculate(MF);
 
diff --git a/llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp b/llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp
index 5f0697f5aaad7..40bc6b180fb88 100644
--- a/llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp
+++ b/llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp
@@ -65,7 +65,7 @@ static bool shouldNotRemoveInstruction(const TargetInstrInfo &TII,
 
 static void extractInstrFromFunction(Oracle &O, MachineFunction &MF) {
   MachineDominatorTree MDT;
-  MDT.calculate(MF);
+  MDT.recalculate(MF);
 
   auto MRI = &MF.getRegInfo();
   SetVector<MachineInstr *> ToDelete;
diff --git a/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp b/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp
index 5838ab6f782ba..85bfd3b37c0f3 100644
--- a/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp
+++ b/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp
@@ -168,8 +168,8 @@ body: |
   WebAssemblyExceptionInfo WEI;
   MachineDominatorTree MDT;
   MachineDominanceFrontier MDF;
-  MDT.calculate(*MF);
-  MDF.getBase().analyze(MDT.getBase());
+  MDT.recalculate(*MF);
+  MDF.getBase().analyze(MDT);
   WEI.recalculate(*MF, MDT, MDF);
 
   // Exception info structure:
@@ -343,8 +343,8 @@ body: |
   WebAssemblyExceptionInfo WEI;
   MachineDominatorTree MDT;
   MachineDominanceFrontier MDF;
-  MDT.calculate(*MF);
-  MDF.getBase().analyze(MDT.getBase());
+  MDT.recalculate(*MF);
+  MDF.getBase().analyze(MDT);
   WEI.recalculate(*MF, MDT, MDF);
 
   // Exception info structure:

>From e953eaacbc527a39d1fe6afdb57e8074a92108b8 Mon Sep 17 00:00:00 2001
From: PaperChalice <liujunchang97 at outlook.com>
Date: Mon, 1 Jul 2024 19:55:53 +0800
Subject: [PATCH 2/2] Move update code to MachineBasicBlock

---
 llvm/lib/CodeGen/MachineBasicBlock.cpp | 14 ++++++++++++++
 llvm/lib/CodeGen/MachineSink.cpp       | 10 ----------
 llvm/lib/CodeGen/PHIElimination.cpp    | 21 ++++-----------------
 3 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index 4f0870fa533d1..e833a11c23582 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -16,11 +16,13 @@
 #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"
@@ -1363,6 +1365,18 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
       }
     }
 
+  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}});
+
   return NMBB;
 }
 
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 7073a90c09652..4dabaabe3659f 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -30,7 +30,6 @@
 #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"
@@ -722,10 +721,6 @@ bool MachineSinking::runOnMachineFunction(MachineFunction &MF) {
   bool EverMadeChange = false;
 
   while (true) {
-    // Ensure that the dominant tree is up-to-date after splitting the critical
-    // edge.
-    MachineDomTreeUpdater MDTU(DT, PDT,
-                               MachineDomTreeUpdater::UpdateStrategy::Lazy);
     bool MadeChange = false;
 
     // Process all basic blocks.
@@ -748,11 +743,6 @@ 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/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 74c3102f9982b..d1f59049c0a40 100644
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -21,7 +21,6 @@
 #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"
@@ -96,8 +95,7 @@ namespace {
     /// Split critical edges where necessary for good coalescer performance.
     bool SplitPHIEdges(MachineFunction &MF, MachineBasicBlock &MBB,
                        MachineLoopInfo *MLI,
-                       std::vector<SparseBitVector<>> *LiveInSets,
-                       MachineDomTreeUpdater *MDTU);
+                       std::vector<SparseBitVector<>> *LiveInSets);
 
     // These functions are temporary abstractions around LiveVariables and
     // LiveIntervals, so they can go away when LiveVariables does.
@@ -185,13 +183,8 @@ 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),
-                               DTWrapper ? &MDTU : nullptr);
+      Changed |= SplitPHIEdges(MF, MBB, MLI, (LV ? &LiveInSets : nullptr));
   }
 
   // This pass takes the function out of SSA form.
@@ -677,8 +670,7 @@ void PHIElimination::analyzePHINodes(const MachineFunction& MF) {
 
 bool PHIElimination::SplitPHIEdges(MachineFunction &MF, MachineBasicBlock &MBB,
                                    MachineLoopInfo *MLI,
-                                   std::vector<SparseBitVector<>> *LiveInSets,
-                                   MachineDomTreeUpdater *MDTU) {
+                                   std::vector<SparseBitVector<>> *LiveInSets) {
   if (MBB.empty() || !MBB.front().isPHI() || MBB.isEHPad())
     return false;   // Quick exit for basic blocks without PHIs.
 
@@ -745,12 +737,7 @@ bool PHIElimination::SplitPHIEdges(MachineFunction &MF, MachineBasicBlock &MBB,
       }
       if (!ShouldSplit && !SplitAllCriticalEdges)
         continue;
-      if (auto *NewMBB = PreMBB->SplitCriticalEdge(&MBB, *this, LiveInSets)) {
-        if (MDTU)
-          MDTU->applyUpdates({{MachineDominatorTree::Insert, PreMBB, NewMBB},
-                              {MachineDominatorTree::Insert, NewMBB, &MBB},
-                              {MachineDominatorTree::Delete, PreMBB, &MBB}});
-      } else {
+      if (!PreMBB->SplitCriticalEdge(&MBB, *this, LiveInSets)) {
         LLVM_DEBUG(dbgs() << "Failed to split critical edge.\n");
         continue;
       }



More information about the llvm-commits mailing list