Revert "[DomTreeUpdater] Move critical edge splitting code to updater" #119512

paperchalice · 2024-12-11T06:23:30Z

Reverts #115111

…#115111)" This reverts commit 79047fa.

llvmbot · 2024-12-11T06:24:03Z

@llvm/pr-subscribers-llvm-regalloc
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-x86

Author: None (paperchalice)

Changes

Reverts llvm/llvm-project#115111

Patch is 57.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119512.diff

30 Files Affected:

(modified) llvm/include/llvm/Analysis/DomTreeUpdater.h (+3-10)
(modified) llvm/include/llvm/Analysis/GenericDomTreeUpdater.h (+7-36)
(modified) llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h (+63-191)
(modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+8-16)
(modified) llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h (-7)
(modified) llvm/include/llvm/CodeGen/MachineDominators.h (+167-2)
(modified) llvm/include/llvm/CodeGen/MachineSSAContext.h (+6)
(modified) llvm/lib/Analysis/DomTreeUpdater.cpp (-7)
(modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+2-2)
(modified) llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp (+2-2)
(modified) llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp (+1-1)
(modified) llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+3-5)
(modified) llvm/lib/CodeGen/MachineDomTreeUpdater.cpp (-7)
(modified) llvm/lib/CodeGen/MachineDominanceFrontier.cpp (+2-1)
(modified) llvm/lib/CodeGen/MachineDominators.cpp (+74)
(modified) llvm/lib/CodeGen/MachineLICM.cpp (+7-10)
(modified) llvm/lib/CodeGen/MachineLoopInfo.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineSink.cpp (+1-5)
(modified) llvm/lib/CodeGen/MachineUniformityAnalysis.cpp (+2-1)
(modified) llvm/lib/CodeGen/PHIElimination.cpp (+9-18)
(modified) llvm/lib/CodeGen/XRayInstrumentation.cpp (+2-2)
(modified) llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp (+3-3)
(modified) llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp (+1-1)
(modified) llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp (+1-1)
(modified) llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp (+1-1)
(modified) llvm/lib/Target/X86/X86FlagsCopyLowering.cpp (+2-1)
(modified) llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp (+1-1)
(modified) llvm/unittests/Analysis/DomTreeUpdaterTest.cpp (-43)
(modified) llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp (+4-4)

diff --git a/llvm/include/llvm/Analysis/DomTreeUpdater.h b/llvm/include/llvm/Analysis/DomTreeUpdater.h
index 0386262ba2b653..c120a6cc6ce5ab 100644
--- a/llvm/include/llvm/Analysis/DomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/DomTreeUpdater.h
@@ -81,9 +81,6 @@ class DomTreeUpdater
 
   ///@}
 
-  /// Debug method to help view the internal state of this class.
-  LLVM_DUMP_METHOD void dump() const;
-
 private:
   class CallBackOnDeletion final : public CallbackVH {
   public:
@@ -112,6 +109,9 @@ class DomTreeUpdater
 
   /// Returns true if at least one BasicBlock is deleted.
   bool forceFlushDeletedBB();
+
+  /// Debug method to help view the internal state of this class.
+  LLVM_DUMP_METHOD void dump() const;
 };
 
 extern template class GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
@@ -120,13 +120,6 @@ extern template class GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
 extern template void
 GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
                       PostDominatorTree>::recalculate(Function &F);
-
-extern template void
-GenericDomTreeUpdater<DomTreeUpdater, DominatorTree, PostDominatorTree>::
-    applyUpdatesImpl</*IsForward=*/true>();
-extern template void
-GenericDomTreeUpdater<DomTreeUpdater, DominatorTree, PostDominatorTree>::
-    applyUpdatesImpl</*IsForward=*/false>();
 } // namespace llvm
 
 #endif // LLVM_ANALYSIS_DOMTREEUPDATER_H
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
index 4a03f548823ee4..ca4ce68b85cbcf 100644
--- a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
@@ -30,7 +30,6 @@ class GenericDomTreeUpdater {
 public:
   enum class UpdateStrategy : unsigned char { Eager = 0, Lazy = 1 };
   using BasicBlockT = typename DomTreeT::NodeType;
-  using UpdateT = typename DomTreeT::UpdateType;
 
   explicit GenericDomTreeUpdater(UpdateStrategy Strategy_)
       : Strategy(Strategy_) {}
@@ -147,12 +146,7 @@ class GenericDomTreeUpdater {
   /// 2. It is illegal to submit any update that has already been submitted,
   /// i.e., you are supposed not to insert an existent edge or delete a
   /// nonexistent edge.
-  void applyUpdates(ArrayRef<UpdateT> Updates);
-
-  /// Apply updates that the critical edge (FromBB, ToBB) has been
-  /// split with NewBB.
-  void splitCriticalEdge(BasicBlockT *FromBB, BasicBlockT *ToBB,
-                         BasicBlockT *NewBB);
+  void applyUpdates(ArrayRef<typename DomTreeT::UpdateType> Updates);
 
   /// Submit updates to all available trees. It will also
   /// 1. discard duplicated updates,
@@ -175,7 +169,7 @@ class GenericDomTreeUpdater {
   /// 3. It is only legal to submit updates to an edge in the order CFG changes
   /// are made. The order you submit updates on different edges is not
   /// restricted.
-  void applyUpdatesPermissive(ArrayRef<UpdateT> Updates);
+  void applyUpdatesPermissive(ArrayRef<typename DomTreeT::UpdateType> Updates);
 
   ///@}
 
@@ -211,25 +205,7 @@ class GenericDomTreeUpdater {
   LLVM_DUMP_METHOD void dump() const;
 
 protected:
-  /// Helper structure used to hold all the basic blocks
-  /// involved in the split of a critical edge.
-  struct CriticalEdge {
-    BasicBlockT *FromBB;
-    BasicBlockT *ToBB;
-    BasicBlockT *NewBB;
-  };
-
-  struct DomTreeUpdate {
-    bool IsCriticalEdgeSplit = false;
-    union {
-      UpdateT Update;
-      CriticalEdge EdgeSplit;
-    };
-    DomTreeUpdate(UpdateT Update) : Update(Update) {}
-    DomTreeUpdate(CriticalEdge E) : IsCriticalEdgeSplit(true), EdgeSplit(E) {}
-  };
-
-  SmallVector<DomTreeUpdate, 16> PendUpdates;
+  SmallVector<typename DomTreeT::UpdateType, 16> PendUpdates;
   size_t PendDTUpdateIndex = 0;
   size_t PendPDTUpdateIndex = 0;
   DomTreeT *DT = nullptr;
@@ -240,21 +216,21 @@ class GenericDomTreeUpdater {
   bool IsRecalculatingPostDomTree = false;
 
   /// Returns true if the update is self dominance.
-  bool isSelfDominance(UpdateT Update) const {
+  bool isSelfDominance(typename DomTreeT::UpdateType Update) const {
     // Won't affect DomTree and PostDomTree.
     return Update.getFrom() == Update.getTo();
   }
 
   /// Helper function to apply all pending DomTree updates.
-  void applyDomTreeUpdates() { applyUpdatesImpl<true>(); }
+  void applyDomTreeUpdates();
 
   /// Helper function to apply all pending PostDomTree updates.
-  void applyPostDomTreeUpdates() { applyUpdatesImpl<false>(); }
+  void applyPostDomTreeUpdates();
 
   /// Returns true if the update appears in the LLVM IR.
   /// It is used to check whether an update is valid in
   /// insertEdge/deleteEdge or is unnecessary in the batch update.
-  bool isUpdateValid(UpdateT Update) const;
+  bool isUpdateValid(typename DomTreeT::UpdateType Update) const;
 
   /// Erase Basic Block node before it is unlinked from Function
   /// in the DomTree and PostDomTree.
@@ -267,11 +243,6 @@ class GenericDomTreeUpdater {
   /// Drop all updates applied by all available trees and delete BasicBlocks if
   /// all available trees are up-to-date.
   void dropOutOfDateUpdates();
-
-private:
-  void splitDTCriticalEdges(ArrayRef<CriticalEdge> Updates);
-  void splitPDTCriticalEdges(ArrayRef<CriticalEdge> Updates);
-  template <bool IsForward> void applyUpdatesImpl();
 };
 
 } // namespace llvm
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
index 70ae72626c6a2a..b79eaef57710fa 100644
--- a/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
+++ b/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
@@ -16,7 +16,6 @@
 #ifndef LLVM_ANALYSIS_GENERICDOMTREEUPDATERIMPL_H
 #define LLVM_ANALYSIS_GENERICDOMTREEUPDATERIMPL_H
 
-#include "llvm/ADT/SmallBitVector.h"
 #include "llvm/Analysis/GenericDomTreeUpdater.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -57,7 +56,7 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::recalculate(
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::applyUpdates(
-    ArrayRef<UpdateT> Updates) {
+    ArrayRef<typename DomTreeT::UpdateType> Updates) {
   if (!DT && !PDT)
     return;
 
@@ -78,12 +77,12 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::applyUpdates(
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
-    applyUpdatesPermissive(ArrayRef<UpdateT> Updates) {
+    applyUpdatesPermissive(ArrayRef<typename DomTreeT::UpdateType> Updates) {
   if (!DT && !PDT)
     return;
 
   SmallSet<std::pair<BasicBlockT *, BasicBlockT *>, 8> Seen;
-  SmallVector<UpdateT, 8> DeduplicatedUpdates;
+  SmallVector<typename DomTreeT::UpdateType, 8> DeduplicatedUpdates;
   for (const auto &U : Updates) {
     auto Edge = std::make_pair(U.getFrom(), U.getTo());
     // Because it is illegal to submit updates that have already been applied
@@ -130,24 +129,6 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
     PDT->applyUpdates(DeduplicatedUpdates);
 }
 
-template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::splitCriticalEdge(
-    BasicBlockT *FromBB, BasicBlockT *ToBB, BasicBlockT *NewBB) {
-  if (!DT && !PDT)
-    return;
-
-  CriticalEdge E = {FromBB, ToBB, NewBB};
-  if (Strategy == UpdateStrategy::Lazy) {
-    PendUpdates.push_back(E);
-    return;
-  }
-
-  if (DT)
-    splitDTCriticalEdges(E);
-  if (PDT)
-    splitPDTCriticalEdges(E);
-}
-
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 DomTreeT &
 GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::getDomTree() {
@@ -188,39 +169,41 @@ GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::dump() const {
     return;
   } else
     OS << "Lazy\n";
-
-  auto printBlockInfo = [&](BasicBlockT *BB, StringRef Ending) {
-    if (BB) {
-      auto S = BB->getName();
-      if (!BB->hasName())
-        S = "(no name)";
-      OS << S << "(" << BB << ")" << Ending;
-    } else {
-      OS << "(badref)" << Ending;
-    }
-  };
+  int Index = 0;
 
   auto printUpdates =
-      [&](typename ArrayRef<DomTreeUpdate>::const_iterator begin,
-          typename ArrayRef<DomTreeUpdate>::const_iterator end) {
+      [&](typename ArrayRef<typename DomTreeT::UpdateType>::const_iterator
+              begin,
+          typename ArrayRef<typename DomTreeT::UpdateType>::const_iterator
+              end) {
         if (begin == end)
           OS << "  None\n";
-        for (auto [Index, DTUpdate] : enumerate(make_range(begin, end))) {
-          if (!DTUpdate.IsCriticalEdgeSplit) {
-            auto U = DTUpdate.Update;
-            OS << "  " << Index << " : ";
-            if (U.getKind() == DomTreeT::Insert)
-              OS << "Insert, ";
-            else
-              OS << "Delete, ";
-            printBlockInfo(U.getFrom(), ", ");
-            printBlockInfo(U.getTo(), "\n");
+        Index = 0;
+        for (auto It = begin, ItEnd = end; It != ItEnd; ++It) {
+          auto U = *It;
+          OS << "  " << Index << " : ";
+          ++Index;
+          if (U.getKind() == DomTreeT::Insert)
+            OS << "Insert, ";
+          else
+            OS << "Delete, ";
+          BasicBlockT *From = U.getFrom();
+          if (From) {
+            auto S = From->getName();
+            if (!From->hasName())
+              S = "(no name)";
+            OS << S << "(" << From << "), ";
           } else {
-            const auto &Edge = DTUpdate.EdgeSplit;
-            OS << "  " << Index << " : Split critical edge, ";
-            printBlockInfo(Edge.FromBB, ", ");
-            printBlockInfo(Edge.ToBB, ", ");
-            printBlockInfo(Edge.NewBB, "\n");
+            OS << "(badref), ";
+          }
+          BasicBlockT *To = U.getTo();
+          if (To) {
+            auto S = To->getName();
+            if (!To->hasName())
+              S = "(no_name)";
+            OS << S << "(" << To << ")\n";
+          } else {
+            OS << "(badref)\n";
           }
         }
       };
@@ -246,58 +229,57 @@ GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::dump() const {
   }
 
   OS << "Pending DeletedBBs:\n";
-  for (auto [Index, BB] : enumerate(DeletedBBs)) {
+  Index = 0;
+  for (const auto *BB : DeletedBBs) {
     OS << "  " << Index << " : ";
+    ++Index;
     if (BB->hasName())
       OS << BB->getName() << "(";
     else
-      OS << "(no name)(";
+      OS << "(no_name)(";
     OS << BB << ")\n";
   }
 #endif
 }
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-template <bool IsForward>
 void GenericDomTreeUpdater<DerivedT, DomTreeT,
-                           PostDomTreeT>::applyUpdatesImpl() {
-  auto *DomTree = [&]() {
-    if constexpr (IsForward)
-      return DT;
-    else
-      return PDT;
-  }();
+                           PostDomTreeT>::applyDomTreeUpdates() {
   // No pending DomTreeUpdates.
-  if (Strategy != UpdateStrategy::Lazy || !DomTree)
+  if (Strategy != UpdateStrategy::Lazy || !DT)
     return;
-  size_t &PendUpdateIndex = IsForward ? PendDTUpdateIndex : PendPDTUpdateIndex;
 
-  // Only apply updates not are applied by (Post)DomTree.
-  while (IsForward ? hasPendingDomTreeUpdates()
-                   : hasPendingPostDomTreeUpdates()) {
-    auto I = PendUpdates.begin() + PendUpdateIndex;
+  // Only apply updates not are applied by DomTree.
+  if (hasPendingDomTreeUpdates()) {
+    const auto I = PendUpdates.begin() + PendDTUpdateIndex;
     const auto E = PendUpdates.end();
     assert(I < E && "Iterator range invalid; there should be DomTree updates.");
-    if (!I->IsCriticalEdgeSplit) {
-      SmallVector<UpdateT, 32> NormalUpdates;
-      for (; I != E && !I->IsCriticalEdgeSplit; ++I)
-        NormalUpdates.push_back(I->Update);
-      DomTree->applyUpdates(NormalUpdates);
-      PendUpdateIndex += NormalUpdates.size();
-    } else {
-      SmallVector<CriticalEdge> CriticalEdges;
-      for (; I != E && I->IsCriticalEdgeSplit; ++I)
-        CriticalEdges.push_back(I->EdgeSplit);
-      IsForward ? splitDTCriticalEdges(CriticalEdges)
-                : splitPDTCriticalEdges(CriticalEdges);
-      PendUpdateIndex += CriticalEdges.size();
-    }
+    DT->applyUpdates(ArrayRef<typename DomTreeT::UpdateType>(I, E));
+    PendDTUpdateIndex = PendUpdates.size();
+  }
+}
+
+template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
+void GenericDomTreeUpdater<DerivedT, DomTreeT,
+                           PostDomTreeT>::applyPostDomTreeUpdates() {
+  // No pending PostDomTreeUpdates.
+  if (Strategy != UpdateStrategy::Lazy || !PDT)
+    return;
+
+  // Only apply updates not are applied by PostDomTree.
+  if (hasPendingPostDomTreeUpdates()) {
+    const auto I = PendUpdates.begin() + PendPDTUpdateIndex;
+    const auto E = PendUpdates.end();
+    assert(I < E &&
+           "Iterator range invalid; there should be PostDomTree updates.");
+    PDT->applyUpdates(ArrayRef<typename DomTreeT::UpdateType>(I, E));
+    PendPDTUpdateIndex = PendUpdates.size();
   }
 }
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 bool GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::isUpdateValid(
-    UpdateT Update) const {
+    typename DomTreeT::UpdateType Update) const {
   const auto *From = Update.getFrom();
   const auto *To = Update.getTo();
   const auto Kind = Update.getKind();
@@ -365,116 +347,6 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT,
   PendPDTUpdateIndex -= dropIndex;
 }
 
-template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
-    splitDTCriticalEdges(ArrayRef<CriticalEdge> Edges) {
-  // Bail out early if there is nothing to do.
-  if (!DT || Edges.empty())
-    return;
-
-  // 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 Edges.
-  // I.e., forall elt in Edges, it exists BB in NewBBs
-  // such as BB == elt.NewBB.
-  SmallSet<BasicBlockT *, 32> NewBBs;
-  for (auto &Edge : Edges)
-    NewBBs.insert(Edge.NewBB);
-  // For each element in Edges, 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 Edges is
-  // the ith element of IsNewIDom.
-  SmallBitVector IsNewIDom(Edges.size(), true);
-
-  // Collect all the dominance properties info, before invalidating
-  // the underlying DT.
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    // Update dominator information.
-    BasicBlockT *Succ = Edge.ToBB;
-    auto *SuccDTNode = DT->getNode(Succ);
-
-    for (BasicBlockT *PredBB : predecessors(Succ)) {
-      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.contains(PredBB)) {
-        assert(pred_size(PredBB) == 1 && "A basic block resulting from a "
-                                         "critical edge split has more "
-                                         "than one predecessor!");
-        PredBB = *pred_begin(PredBB);
-      }
-      if (!DT->dominates(SuccDTNode, DT->getNode(PredBB))) {
-        IsNewIDom[Idx] = false;
-        break;
-      }
-    }
-  }
-
-  // Now, update DT with the collected dominance properties info.
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    // We know FromBB dominates NewBB.
-    auto *NewDTNode = DT->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])
-      DT->changeImmediateDominator(DT->getNode(Edge.ToBB), NewDTNode);
-  }
-}
-
-template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
-    splitPDTCriticalEdges(ArrayRef<CriticalEdge> Edges) {
-  // Bail out early if there is nothing to do.
-  if (!PDT || Edges.empty())
-    return;
-
-  SmallBitVector IsNewIPDom(Edges.size(), true);
-  SmallSet<BasicBlockT *, 8> NewBBs;
-  for (const auto &Edge : Edges)
-    NewBBs.insert(Edge.NewBB);
-
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    // Same as DT version but from another direction.
-    BasicBlockT *Pred = Edge.FromBB;
-    auto *PredDTNode = PDT->getNode(Pred);
-    for (BasicBlockT *SuccBB : successors(Pred)) {
-      if (SuccBB == Edge.NewBB)
-        continue;
-      if (NewBBs.count(SuccBB)) {
-        assert(succ_size(SuccBB) == 1 && "A basic block resulting from a "
-                                         "critical edge split has more "
-                                         "than one successor!");
-        SuccBB = *succ_begin(SuccBB);
-      }
-      if (!PDT->dominates(PredDTNode, PDT->getNode(SuccBB))) {
-        IsNewIPDom[Idx] = false;
-        break;
-      }
-    }
-  }
-
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    auto *NewPDTNode = PDT->addNewBlock(Edge.NewBB, Edge.ToBB);
-    if (IsNewIPDom[Idx])
-      PDT->changeImmediateDominator(PDT->getNode(Edge.FromBB), NewPDTNode);
-  }
-}
-
 } // namespace llvm
 
 #endif // LLVM_ANALYSIS_GENERICDOMTREEUPDATERIMPL_H
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 7fe33c3913f2dd..6cf151c951b19f 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -32,7 +32,6 @@
 namespace llvm {
 
 class BasicBlock;
-class MachineDomTreeUpdater;
 class MachineFunction;
 class MCSymbol;
 class ModuleSlotTracker;
@@ -973,23 +972,22 @@ class MachineBasicBlock
   /// MachineLoopInfo, as applicable.
   MachineBasicBlock *
   SplitCriticalEdge(MachineBasicBlock *Succ, Pass &P,
-                    std::vector<SparseBitVector<>> *LiveInSets = nullptr,
-                    MachineDomTreeUpdater *MDTU = nullptr) {
-    return SplitCriticalEdge(Succ, &P, nullptr, LiveInSets, MDTU);
+                    std::vector<SparseBitVector<>> *LiveInSets = nullptr) {
+    return SplitCriticalEdge(Succ, &P, nullptr, LiveInSets);
   }
 
   MachineBasicBlock *
   SplitCriticalEdge(MachineBasicBlock *Succ,
                     MachineFunctionAnalysisManager &MFAM,
-                    std::vector<SparseBitVector<>> *LiveInSets = nullptr,
-                    MachineDomTreeUpdater *MDTU = nullptr) {
-    return SplitCriticalEdge(Succ, nullptr, &MFAM, LiveInSets, MDTU);
+                    std::vector<SparseBitVector<>> *LiveInSets = nullptr) {
+    return SplitCriticalEdge(Succ, nullptr, &MFAM, LiveInSets);
   }
 
   // Helper method for new pass manager migration.
-  MachineBasicBlock *SplitCriticalEdge(
-      MachineBasicBlock *Succ, Pass *P, MachineFunctionAnalysisManager *MFAM,
-      std::vector<SparseBitVector<>> *LiveInSets, MachineDomTreeUpdater *MDTU);
+  MachineBasicBlock *
+  SplitCriticalEdge(MachineBasicBlock *Succ, Pass *P,
+                    MachineFunctionAnalysisManager *MFAM,
+                    std::vector<SparseBitVector<>> *LiveInSets);
 
   /// Check if the edge between this block and the given successor \p
   /// Succ, can be split. If this returns true a subsequent call to
@@ -1377,12 +1375,6 @@ inline auto successors(const MachineBasicBlock *BB) { return BB->successors(); }
 inline auto predecessors(const MachineBasicBlock *BB) {
   return BB->predecessors();
 }
-inline auto succ_size(const MachineBasicBlock *BB) { return BB->succ_size(); }
-inline auto pred_size(const MachineBasicBlock *BB) { return BB->pred_size(); }
-inline auto succ_begin(const MachineBasicBlock *BB) { return BB->succ_begin(); }
-inline auto pred_begin(const MachineBasicBlock *BB) { return BB->pr...
[truncated]

llvmbot · 2024-12-11T06:24:04Z

@llvm/pr-subscribers-backend-hexagon

Author: None (paperchalice)

Changes

Reverts llvm/llvm-project#115111

Patch is 57.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119512.diff

30 Files Affected:

(modified) llvm/include/llvm/Analysis/DomTreeUpdater.h (+3-10)
(modified) llvm/include/llvm/Analysis/GenericDomTreeUpdater.h (+7-36)
(modified) llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h (+63-191)
(modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+8-16)
(modified) llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h (-7)
(modified) llvm/include/llvm/CodeGen/MachineDominators.h (+167-2)
(modified) llvm/include/llvm/CodeGen/MachineSSAContext.h (+6)
(modified) llvm/lib/Analysis/DomTreeUpdater.cpp (-7)
(modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+2-2)
(modified) llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp (+2-2)
(modified) llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp (+1-1)
(modified) llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+3-5)
(modified) llvm/lib/CodeGen/MachineDomTreeUpdater.cpp (-7)
(modified) llvm/lib/CodeGen/MachineDominanceFrontier.cpp (+2-1)
(modified) llvm/lib/CodeGen/MachineDominators.cpp (+74)
(modified) llvm/lib/CodeGen/MachineLICM.cpp (+7-10)
(modified) llvm/lib/CodeGen/MachineLoopInfo.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineSink.cpp (+1-5)
(modified) llvm/lib/CodeGen/MachineUniformityAnalysis.cpp (+2-1)
(modified) llvm/lib/CodeGen/PHIElimination.cpp (+9-18)
(modified) llvm/lib/CodeGen/XRayInstrumentation.cpp (+2-2)
(modified) llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp (+3-3)
(modified) llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp (+1-1)
(modified) llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp (+1-1)
(modified) llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp (+1-1)
(modified) llvm/lib/Target/X86/X86FlagsCopyLowering.cpp (+2-1)
(modified) llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp (+1-1)
(modified) llvm/unittests/Analysis/DomTreeUpdaterTest.cpp (-43)
(modified) llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp (+4-4)

diff --git a/llvm/include/llvm/Analysis/DomTreeUpdater.h b/llvm/include/llvm/Analysis/DomTreeUpdater.h
index 0386262ba2b653..c120a6cc6ce5ab 100644
--- a/llvm/include/llvm/Analysis/DomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/DomTreeUpdater.h
@@ -81,9 +81,6 @@ class DomTreeUpdater
 
   ///@}
 
-  /// Debug method to help view the internal state of this class.
-  LLVM_DUMP_METHOD void dump() const;
-
 private:
   class CallBackOnDeletion final : public CallbackVH {
   public:
@@ -112,6 +109,9 @@ class DomTreeUpdater
 
   /// Returns true if at least one BasicBlock is deleted.
   bool forceFlushDeletedBB();
+
+  /// Debug method to help view the internal state of this class.
+  LLVM_DUMP_METHOD void dump() const;
 };
 
 extern template class GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
@@ -120,13 +120,6 @@ extern template class GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
 extern template void
 GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
                       PostDominatorTree>::recalculate(Function &F);
-
-extern template void
-GenericDomTreeUpdater<DomTreeUpdater, DominatorTree, PostDominatorTree>::
-    applyUpdatesImpl</*IsForward=*/true>();
-extern template void
-GenericDomTreeUpdater<DomTreeUpdater, DominatorTree, PostDominatorTree>::
-    applyUpdatesImpl</*IsForward=*/false>();
 } // namespace llvm
 
 #endif // LLVM_ANALYSIS_DOMTREEUPDATER_H
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
index 4a03f548823ee4..ca4ce68b85cbcf 100644
--- a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
@@ -30,7 +30,6 @@ class GenericDomTreeUpdater {
 public:
   enum class UpdateStrategy : unsigned char { Eager = 0, Lazy = 1 };
   using BasicBlockT = typename DomTreeT::NodeType;
-  using UpdateT = typename DomTreeT::UpdateType;
 
   explicit GenericDomTreeUpdater(UpdateStrategy Strategy_)
       : Strategy(Strategy_) {}
@@ -147,12 +146,7 @@ class GenericDomTreeUpdater {
   /// 2. It is illegal to submit any update that has already been submitted,
   /// i.e., you are supposed not to insert an existent edge or delete a
   /// nonexistent edge.
-  void applyUpdates(ArrayRef<UpdateT> Updates);
-
-  /// Apply updates that the critical edge (FromBB, ToBB) has been
-  /// split with NewBB.
-  void splitCriticalEdge(BasicBlockT *FromBB, BasicBlockT *ToBB,
-                         BasicBlockT *NewBB);
+  void applyUpdates(ArrayRef<typename DomTreeT::UpdateType> Updates);
 
   /// Submit updates to all available trees. It will also
   /// 1. discard duplicated updates,
@@ -175,7 +169,7 @@ class GenericDomTreeUpdater {
   /// 3. It is only legal to submit updates to an edge in the order CFG changes
   /// are made. The order you submit updates on different edges is not
   /// restricted.
-  void applyUpdatesPermissive(ArrayRef<UpdateT> Updates);
+  void applyUpdatesPermissive(ArrayRef<typename DomTreeT::UpdateType> Updates);
 
   ///@}
 
@@ -211,25 +205,7 @@ class GenericDomTreeUpdater {
   LLVM_DUMP_METHOD void dump() const;
 
 protected:
-  /// Helper structure used to hold all the basic blocks
-  /// involved in the split of a critical edge.
-  struct CriticalEdge {
-    BasicBlockT *FromBB;
-    BasicBlockT *ToBB;
-    BasicBlockT *NewBB;
-  };
-
-  struct DomTreeUpdate {
-    bool IsCriticalEdgeSplit = false;
-    union {
-      UpdateT Update;
-      CriticalEdge EdgeSplit;
-    };
-    DomTreeUpdate(UpdateT Update) : Update(Update) {}
-    DomTreeUpdate(CriticalEdge E) : IsCriticalEdgeSplit(true), EdgeSplit(E) {}
-  };
-
-  SmallVector<DomTreeUpdate, 16> PendUpdates;
+  SmallVector<typename DomTreeT::UpdateType, 16> PendUpdates;
   size_t PendDTUpdateIndex = 0;
   size_t PendPDTUpdateIndex = 0;
   DomTreeT *DT = nullptr;
@@ -240,21 +216,21 @@ class GenericDomTreeUpdater {
   bool IsRecalculatingPostDomTree = false;
 
   /// Returns true if the update is self dominance.
-  bool isSelfDominance(UpdateT Update) const {
+  bool isSelfDominance(typename DomTreeT::UpdateType Update) const {
     // Won't affect DomTree and PostDomTree.
     return Update.getFrom() == Update.getTo();
   }
 
   /// Helper function to apply all pending DomTree updates.
-  void applyDomTreeUpdates() { applyUpdatesImpl<true>(); }
+  void applyDomTreeUpdates();
 
   /// Helper function to apply all pending PostDomTree updates.
-  void applyPostDomTreeUpdates() { applyUpdatesImpl<false>(); }
+  void applyPostDomTreeUpdates();
 
   /// Returns true if the update appears in the LLVM IR.
   /// It is used to check whether an update is valid in
   /// insertEdge/deleteEdge or is unnecessary in the batch update.
-  bool isUpdateValid(UpdateT Update) const;
+  bool isUpdateValid(typename DomTreeT::UpdateType Update) const;
 
   /// Erase Basic Block node before it is unlinked from Function
   /// in the DomTree and PostDomTree.
@@ -267,11 +243,6 @@ class GenericDomTreeUpdater {
   /// Drop all updates applied by all available trees and delete BasicBlocks if
   /// all available trees are up-to-date.
   void dropOutOfDateUpdates();
-
-private:
-  void splitDTCriticalEdges(ArrayRef<CriticalEdge> Updates);
-  void splitPDTCriticalEdges(ArrayRef<CriticalEdge> Updates);
-  template <bool IsForward> void applyUpdatesImpl();
 };
 
 } // namespace llvm
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
index 70ae72626c6a2a..b79eaef57710fa 100644
--- a/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
+++ b/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
@@ -16,7 +16,6 @@
 #ifndef LLVM_ANALYSIS_GENERICDOMTREEUPDATERIMPL_H
 #define LLVM_ANALYSIS_GENERICDOMTREEUPDATERIMPL_H
 
-#include "llvm/ADT/SmallBitVector.h"
 #include "llvm/Analysis/GenericDomTreeUpdater.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -57,7 +56,7 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::recalculate(
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::applyUpdates(
-    ArrayRef<UpdateT> Updates) {
+    ArrayRef<typename DomTreeT::UpdateType> Updates) {
   if (!DT && !PDT)
     return;
 
@@ -78,12 +77,12 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::applyUpdates(
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
-    applyUpdatesPermissive(ArrayRef<UpdateT> Updates) {
+    applyUpdatesPermissive(ArrayRef<typename DomTreeT::UpdateType> Updates) {
   if (!DT && !PDT)
     return;
 
   SmallSet<std::pair<BasicBlockT *, BasicBlockT *>, 8> Seen;
-  SmallVector<UpdateT, 8> DeduplicatedUpdates;
+  SmallVector<typename DomTreeT::UpdateType, 8> DeduplicatedUpdates;
   for (const auto &U : Updates) {
     auto Edge = std::make_pair(U.getFrom(), U.getTo());
     // Because it is illegal to submit updates that have already been applied
@@ -130,24 +129,6 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
     PDT->applyUpdates(DeduplicatedUpdates);
 }
 
-template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::splitCriticalEdge(
-    BasicBlockT *FromBB, BasicBlockT *ToBB, BasicBlockT *NewBB) {
-  if (!DT && !PDT)
-    return;
-
-  CriticalEdge E = {FromBB, ToBB, NewBB};
-  if (Strategy == UpdateStrategy::Lazy) {
-    PendUpdates.push_back(E);
-    return;
-  }
-
-  if (DT)
-    splitDTCriticalEdges(E);
-  if (PDT)
-    splitPDTCriticalEdges(E);
-}
-
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 DomTreeT &
 GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::getDomTree() {
@@ -188,39 +169,41 @@ GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::dump() const {
     return;
   } else
     OS << "Lazy\n";
-
-  auto printBlockInfo = [&](BasicBlockT *BB, StringRef Ending) {
-    if (BB) {
-      auto S = BB->getName();
-      if (!BB->hasName())
-        S = "(no name)";
-      OS << S << "(" << BB << ")" << Ending;
-    } else {
-      OS << "(badref)" << Ending;
-    }
-  };
+  int Index = 0;
 
   auto printUpdates =
-      [&](typename ArrayRef<DomTreeUpdate>::const_iterator begin,
-          typename ArrayRef<DomTreeUpdate>::const_iterator end) {
+      [&](typename ArrayRef<typename DomTreeT::UpdateType>::const_iterator
+              begin,
+          typename ArrayRef<typename DomTreeT::UpdateType>::const_iterator
+              end) {
         if (begin == end)
           OS << "  None\n";
-        for (auto [Index, DTUpdate] : enumerate(make_range(begin, end))) {
-          if (!DTUpdate.IsCriticalEdgeSplit) {
-            auto U = DTUpdate.Update;
-            OS << "  " << Index << " : ";
-            if (U.getKind() == DomTreeT::Insert)
-              OS << "Insert, ";
-            else
-              OS << "Delete, ";
-            printBlockInfo(U.getFrom(), ", ");
-            printBlockInfo(U.getTo(), "\n");
+        Index = 0;
+        for (auto It = begin, ItEnd = end; It != ItEnd; ++It) {
+          auto U = *It;
+          OS << "  " << Index << " : ";
+          ++Index;
+          if (U.getKind() == DomTreeT::Insert)
+            OS << "Insert, ";
+          else
+            OS << "Delete, ";
+          BasicBlockT *From = U.getFrom();
+          if (From) {
+            auto S = From->getName();
+            if (!From->hasName())
+              S = "(no name)";
+            OS << S << "(" << From << "), ";
           } else {
-            const auto &Edge = DTUpdate.EdgeSplit;
-            OS << "  " << Index << " : Split critical edge, ";
-            printBlockInfo(Edge.FromBB, ", ");
-            printBlockInfo(Edge.ToBB, ", ");
-            printBlockInfo(Edge.NewBB, "\n");
+            OS << "(badref), ";
+          }
+          BasicBlockT *To = U.getTo();
+          if (To) {
+            auto S = To->getName();
+            if (!To->hasName())
+              S = "(no_name)";
+            OS << S << "(" << To << ")\n";
+          } else {
+            OS << "(badref)\n";
           }
         }
       };
@@ -246,58 +229,57 @@ GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::dump() const {
   }
 
   OS << "Pending DeletedBBs:\n";
-  for (auto [Index, BB] : enumerate(DeletedBBs)) {
+  Index = 0;
+  for (const auto *BB : DeletedBBs) {
     OS << "  " << Index << " : ";
+    ++Index;
     if (BB->hasName())
       OS << BB->getName() << "(";
     else
-      OS << "(no name)(";
+      OS << "(no_name)(";
     OS << BB << ")\n";
   }
 #endif
 }
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-template <bool IsForward>
 void GenericDomTreeUpdater<DerivedT, DomTreeT,
-                           PostDomTreeT>::applyUpdatesImpl() {
-  auto *DomTree = [&]() {
-    if constexpr (IsForward)
-      return DT;
-    else
-      return PDT;
-  }();
+                           PostDomTreeT>::applyDomTreeUpdates() {
   // No pending DomTreeUpdates.
-  if (Strategy != UpdateStrategy::Lazy || !DomTree)
+  if (Strategy != UpdateStrategy::Lazy || !DT)
     return;
-  size_t &PendUpdateIndex = IsForward ? PendDTUpdateIndex : PendPDTUpdateIndex;
 
-  // Only apply updates not are applied by (Post)DomTree.
-  while (IsForward ? hasPendingDomTreeUpdates()
-                   : hasPendingPostDomTreeUpdates()) {
-    auto I = PendUpdates.begin() + PendUpdateIndex;
+  // Only apply updates not are applied by DomTree.
+  if (hasPendingDomTreeUpdates()) {
+    const auto I = PendUpdates.begin() + PendDTUpdateIndex;
     const auto E = PendUpdates.end();
     assert(I < E && "Iterator range invalid; there should be DomTree updates.");
-    if (!I->IsCriticalEdgeSplit) {
-      SmallVector<UpdateT, 32> NormalUpdates;
-      for (; I != E && !I->IsCriticalEdgeSplit; ++I)
-        NormalUpdates.push_back(I->Update);
-      DomTree->applyUpdates(NormalUpdates);
-      PendUpdateIndex += NormalUpdates.size();
-    } else {
-      SmallVector<CriticalEdge> CriticalEdges;
-      for (; I != E && I->IsCriticalEdgeSplit; ++I)
-        CriticalEdges.push_back(I->EdgeSplit);
-      IsForward ? splitDTCriticalEdges(CriticalEdges)
-                : splitPDTCriticalEdges(CriticalEdges);
-      PendUpdateIndex += CriticalEdges.size();
-    }
+    DT->applyUpdates(ArrayRef<typename DomTreeT::UpdateType>(I, E));
+    PendDTUpdateIndex = PendUpdates.size();
+  }
+}
+
+template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
+void GenericDomTreeUpdater<DerivedT, DomTreeT,
+                           PostDomTreeT>::applyPostDomTreeUpdates() {
+  // No pending PostDomTreeUpdates.
+  if (Strategy != UpdateStrategy::Lazy || !PDT)
+    return;
+
+  // Only apply updates not are applied by PostDomTree.
+  if (hasPendingPostDomTreeUpdates()) {
+    const auto I = PendUpdates.begin() + PendPDTUpdateIndex;
+    const auto E = PendUpdates.end();
+    assert(I < E &&
+           "Iterator range invalid; there should be PostDomTree updates.");
+    PDT->applyUpdates(ArrayRef<typename DomTreeT::UpdateType>(I, E));
+    PendPDTUpdateIndex = PendUpdates.size();
   }
 }
 
 template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
 bool GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::isUpdateValid(
-    UpdateT Update) const {
+    typename DomTreeT::UpdateType Update) const {
   const auto *From = Update.getFrom();
   const auto *To = Update.getTo();
   const auto Kind = Update.getKind();
@@ -365,116 +347,6 @@ void GenericDomTreeUpdater<DerivedT, DomTreeT,
   PendPDTUpdateIndex -= dropIndex;
 }
 
-template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
-    splitDTCriticalEdges(ArrayRef<CriticalEdge> Edges) {
-  // Bail out early if there is nothing to do.
-  if (!DT || Edges.empty())
-    return;
-
-  // 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 Edges.
-  // I.e., forall elt in Edges, it exists BB in NewBBs
-  // such as BB == elt.NewBB.
-  SmallSet<BasicBlockT *, 32> NewBBs;
-  for (auto &Edge : Edges)
-    NewBBs.insert(Edge.NewBB);
-  // For each element in Edges, 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 Edges is
-  // the ith element of IsNewIDom.
-  SmallBitVector IsNewIDom(Edges.size(), true);
-
-  // Collect all the dominance properties info, before invalidating
-  // the underlying DT.
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    // Update dominator information.
-    BasicBlockT *Succ = Edge.ToBB;
-    auto *SuccDTNode = DT->getNode(Succ);
-
-    for (BasicBlockT *PredBB : predecessors(Succ)) {
-      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.contains(PredBB)) {
-        assert(pred_size(PredBB) == 1 && "A basic block resulting from a "
-                                         "critical edge split has more "
-                                         "than one predecessor!");
-        PredBB = *pred_begin(PredBB);
-      }
-      if (!DT->dominates(SuccDTNode, DT->getNode(PredBB))) {
-        IsNewIDom[Idx] = false;
-        break;
-      }
-    }
-  }
-
-  // Now, update DT with the collected dominance properties info.
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    // We know FromBB dominates NewBB.
-    auto *NewDTNode = DT->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])
-      DT->changeImmediateDominator(DT->getNode(Edge.ToBB), NewDTNode);
-  }
-}
-
-template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
-void GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::
-    splitPDTCriticalEdges(ArrayRef<CriticalEdge> Edges) {
-  // Bail out early if there is nothing to do.
-  if (!PDT || Edges.empty())
-    return;
-
-  SmallBitVector IsNewIPDom(Edges.size(), true);
-  SmallSet<BasicBlockT *, 8> NewBBs;
-  for (const auto &Edge : Edges)
-    NewBBs.insert(Edge.NewBB);
-
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    // Same as DT version but from another direction.
-    BasicBlockT *Pred = Edge.FromBB;
-    auto *PredDTNode = PDT->getNode(Pred);
-    for (BasicBlockT *SuccBB : successors(Pred)) {
-      if (SuccBB == Edge.NewBB)
-        continue;
-      if (NewBBs.count(SuccBB)) {
-        assert(succ_size(SuccBB) == 1 && "A basic block resulting from a "
-                                         "critical edge split has more "
-                                         "than one successor!");
-        SuccBB = *succ_begin(SuccBB);
-      }
-      if (!PDT->dominates(PredDTNode, PDT->getNode(SuccBB))) {
-        IsNewIPDom[Idx] = false;
-        break;
-      }
-    }
-  }
-
-  for (const auto &[Idx, Edge] : enumerate(Edges)) {
-    auto *NewPDTNode = PDT->addNewBlock(Edge.NewBB, Edge.ToBB);
-    if (IsNewIPDom[Idx])
-      PDT->changeImmediateDominator(PDT->getNode(Edge.FromBB), NewPDTNode);
-  }
-}
-
 } // namespace llvm
 
 #endif // LLVM_ANALYSIS_GENERICDOMTREEUPDATERIMPL_H
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 7fe33c3913f2dd..6cf151c951b19f 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -32,7 +32,6 @@
 namespace llvm {
 
 class BasicBlock;
-class MachineDomTreeUpdater;
 class MachineFunction;
 class MCSymbol;
 class ModuleSlotTracker;
@@ -973,23 +972,22 @@ class MachineBasicBlock
   /// MachineLoopInfo, as applicable.
   MachineBasicBlock *
   SplitCriticalEdge(MachineBasicBlock *Succ, Pass &P,
-                    std::vector<SparseBitVector<>> *LiveInSets = nullptr,
-                    MachineDomTreeUpdater *MDTU = nullptr) {
-    return SplitCriticalEdge(Succ, &P, nullptr, LiveInSets, MDTU);
+                    std::vector<SparseBitVector<>> *LiveInSets = nullptr) {
+    return SplitCriticalEdge(Succ, &P, nullptr, LiveInSets);
   }
 
   MachineBasicBlock *
   SplitCriticalEdge(MachineBasicBlock *Succ,
                     MachineFunctionAnalysisManager &MFAM,
-                    std::vector<SparseBitVector<>> *LiveInSets = nullptr,
-                    MachineDomTreeUpdater *MDTU = nullptr) {
-    return SplitCriticalEdge(Succ, nullptr, &MFAM, LiveInSets, MDTU);
+                    std::vector<SparseBitVector<>> *LiveInSets = nullptr) {
+    return SplitCriticalEdge(Succ, nullptr, &MFAM, LiveInSets);
   }
 
   // Helper method for new pass manager migration.
-  MachineBasicBlock *SplitCriticalEdge(
-      MachineBasicBlock *Succ, Pass *P, MachineFunctionAnalysisManager *MFAM,
-      std::vector<SparseBitVector<>> *LiveInSets, MachineDomTreeUpdater *MDTU);
+  MachineBasicBlock *
+  SplitCriticalEdge(MachineBasicBlock *Succ, Pass *P,
+                    MachineFunctionAnalysisManager *MFAM,
+                    std::vector<SparseBitVector<>> *LiveInSets);
 
   /// Check if the edge between this block and the given successor \p
   /// Succ, can be split. If this returns true a subsequent call to
@@ -1377,12 +1375,6 @@ inline auto successors(const MachineBasicBlock *BB) { return BB->successors(); }
 inline auto predecessors(const MachineBasicBlock *BB) {
   return BB->predecessors();
 }
-inline auto succ_size(const MachineBasicBlock *BB) { return BB->succ_size(); }
-inline auto pred_size(const MachineBasicBlock *BB) { return BB->pred_size(); }
-inline auto succ_begin(const MachineBasicBlock *BB) { return BB->succ_begin(); }
-inline auto pred_begin(const MachineBasicBlock *BB) { return BB->pr...
[truncated]

Revert "[DomTreeUpdater] Move critical edge splitting code to updater (…

ccd250c

…#115111)" This reverts commit 79047fa.

llvmbot added backend:AMDGPU backend:Hexagon backend:WebAssembly backend:X86 llvm:regalloc debuginfo llvm:analysis Includes value tracking, cost tables and constant folding labels Dec 11, 2024

paperchalice added skip-precommit-approval PR for CI feedback, not intended for review and removed backend:AMDGPU backend:Hexagon backend:WebAssembly backend:X86 llvm:regalloc debuginfo llvm:analysis Includes value tracking, cost tables and constant folding labels Dec 11, 2024

paperchalice merged commit 553058f into main Dec 11, 2024
13 of 17 checks passed

paperchalice deleted the revert-115111-dt-critical-edge branch December 11, 2024 06:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert "[DomTreeUpdater] Move critical edge splitting code to updater" #119512

Revert "[DomTreeUpdater] Move critical edge splitting code to updater" #119512

Uh oh!

paperchalice commented Dec 11, 2024

Uh oh!

llvmbot commented Dec 11, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 11, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Revert "[DomTreeUpdater] Move critical edge splitting code to updater" #119512

Revert "[DomTreeUpdater] Move critical edge splitting code to updater" #119512

Uh oh!

Conversation

paperchalice commented Dec 11, 2024

Uh oh!

llvmbot commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 11, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

llvmbot commented Dec 11, 2024 •

edited

Loading