Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CodeGen][NewPM] Split MachineDominatorTree into a concrete analysis result #94571

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

paperchalice
Copy link
Contributor

Prepare for new pass manager version of MachineDominatorTreeAnalysis. We may need a machine dominator tree version of DomTreeUpdater to handle SplitCriticalEdge in some CodeGen passes.

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 6, 2024

@llvm/pr-subscribers-llvm-regalloc
@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-systemz
@llvm/pr-subscribers-backend-webassembly
@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-backend-x86

Author: None (paperchalice)

Changes

Prepare for new pass manager version of MachineDominatorTreeAnalysis. We may need a machine dominator tree version of DomTreeUpdater to handle SplitCriticalEdge in some CodeGen passes.


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

88 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachineDominators.h (+75-39)
  • (modified) llvm/include/llvm/InitializePasses.h (+1-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+2-1)
  • (modified) llvm/lib/CodeGen/CodeGen.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/EarlyIfConversion.cpp (+8-8)
  • (modified) llvm/lib/CodeGen/InlineSpiller.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp (+2-1)
  • (modified) llvm/lib/CodeGen/LiveDebugVariables.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/LiveIntervals.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/MIRSampleProfile.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineCSE.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/MachineCombiner.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/MachineDominanceFrontier.cpp (+4-3)
  • (modified) llvm/lib/CodeGen/MachineDominators.cpp (+45-32)
  • (modified) llvm/lib/CodeGen/MachineLICM.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/MachineLoopInfo.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachinePipeliner.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineRegionInfo.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineScheduler.cpp (+5-5)
  • (modified) llvm/lib/CodeGen/MachineSink.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineUniformityAnalysis.cpp (+4-3)
  • (modified) llvm/lib/CodeGen/PHIElimination.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/PeepholeOptimizer.cpp (+5-4)
  • (modified) llvm/lib/CodeGen/PostRASchedulerList.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/PrologEpilogInserter.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/RegAllocBasic.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/RegAllocGreedy.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/RegAllocPBQP.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/ShrinkWrap.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/UnreachableBlockElim.cpp (+4-2)
  • (modified) llvm/lib/CodeGen/XRayInstrumentation.cpp (+4-2)
  • (modified) llvm/lib/Target/AArch64/AArch64CleanupLocalDynamicTLSPass.cpp (+3-2)
  • (modified) llvm/lib/Target/AArch64/AArch64ConditionOptimizer.cpp (+4-4)
  • (modified) llvm/lib/Target/AArch64/AArch64ConditionalCompares.cpp (+4-4)
  • (modified) llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUGlobalISelDivergenceLowering.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/R600MachineCFGStructurizer.cpp (+3-3)
  • (modified) llvm/lib/Target/AMDGPU/R600OptimizeVectorRegisters.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/R600Packetizer.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp (+4-5)
  • (modified) llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp (+4-4)
  • (modified) llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp (+5-4)
  • (modified) llvm/lib/Target/AMDGPU/SIOptimizeVGPRLiveRange.cpp (+4-4)
  • (modified) llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp (+2-2)
  • (modified) llvm/lib/Target/ARC/ARCBranchFinalize.cpp (+1-1)
  • (modified) llvm/lib/Target/ARC/ARCOptAddrMode.cpp (+4-4)
  • (modified) llvm/lib/Target/ARM/ARMConstantIslandPass.cpp (+2-2)
  • (modified) llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (+4-4)
  • (modified) llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp (+5-4)
  • (modified) llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp (+1-1)
  • (modified) llvm/lib/Target/Hexagon/HexagonBitSimplify.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonConstExtenders.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp (+2-2)
  • (modified) llvm/lib/Target/Hexagon/HexagonEarlyIfConv.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonGenInsert.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonGenMemAbsolute.cpp (+4-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonGenPredicate.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonHardwareLoops.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonOptAddrMode.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonRDFOpt.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp (+3-3)
  • (modified) llvm/lib/Target/Mips/MipsOptimizePICCall.cpp (+3-2)
  • (modified) llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/PowerPC/PPCBranchCoalescing.cpp (+3-3)
  • (modified) llvm/lib/Target/PowerPC/PPCCTRLoopsVerify.cpp (+3-3)
  • (modified) llvm/lib/Target/PowerPC/PPCMIPeephole.cpp (+4-4)
  • (modified) llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp (+2-2)
  • (modified) llvm/lib/Target/PowerPC/PPCVSXFMAMutate.cpp (+3-3)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/SystemZ/SystemZLDCleanup.cpp (+3-2)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyMemIntrinsicResults.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp (+3-3)
  • (modified) llvm/lib/Target/X86/X86FlagsCopyLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/X86/X86InstrInfo.cpp (+3-2)
  • (modified) llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp (+3-3)
diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 30c18ef410fab..d5231ad2ee3db 100644
--- a/llvm/include/llvm/CodeGen/MachineDominators.h
+++ b/llvm/include/llvm/CodeGen/MachineDominators.h
@@ -44,11 +44,37 @@ extern template class DominatorTreeBase<MachineBasicBlock, true>; // PostDomTree
 using MachineDomTree = DomTreeBase<MachineBasicBlock>;
 using MachineDomTreeNode = DomTreeNodeBase<MachineBasicBlock>;
 
+namespace DomTreeBuilder {
+using MBBDomTree = MachineDomTree;
+using MBBUpdates = ArrayRef<llvm::cfg::Update<MachineBasicBlock *>>;
+using MBBDomTreeGraphDiff = GraphDiff<MachineBasicBlock *, false>;
+
+extern template void Calculate<MBBDomTree>(MBBDomTree &DT);
+extern template void CalculateWithUpdates<MBBDomTree>(MBBDomTree &DT,
+                                                      MBBUpdates U);
+
+extern template void InsertEdge<MBBDomTree>(MBBDomTree &DT,
+                                            MachineBasicBlock *From,
+                                            MachineBasicBlock *To);
+
+extern template void DeleteEdge<MBBDomTree>(MBBDomTree &DT,
+                                            MachineBasicBlock *From,
+                                            MachineBasicBlock *To);
+
+extern template void ApplyUpdates<MBBDomTree>(MBBDomTree &DT,
+                                              MBBDomTreeGraphDiff &,
+                                              MBBDomTreeGraphDiff *);
+
+extern template bool Verify<MBBDomTree>(const MBBDomTree &DT,
+                                        MBBDomTree::VerificationLevel VL);
+} // namespace DomTreeBuilder
+
 //===-------------------------------------
 /// DominatorTree Class - Concrete subclass of DominatorTreeBase that is used to
 /// compute a normal dominator tree.
 ///
-class MachineDominatorTree : public MachineFunctionPass {
+class MachineDominatorTree : public MachineDomTree {
+  friend class MachineDominatorTreeWrapperPass;
   /// Helper structure used to hold all the basic blocks
   /// involved in the split of a critical edge.
   struct CriticalEdge {
@@ -70,62 +96,52 @@ class MachineDominatorTree : public MachineFunctionPass {
   /// such as BB == elt.NewBB.
   mutable SmallSet<MachineBasicBlock *, 32> NewBBs;
 
-  /// The DominatorTreeBase that is used to compute a normal dominator tree.
-  std::unique_ptr<MachineDomTree> DT;
-
   /// 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:
-  static char ID; // Pass ID, replacement for typeid
+  using Base = MachineDomTree;
 
-  MachineDominatorTree();
-  explicit MachineDominatorTree(MachineFunction &MF) : MachineFunctionPass(ID) {
-    calculate(MF);
-  }
+  MachineDominatorTree() = default;
+  explicit MachineDominatorTree(MachineFunction &MF) { calculate(MF); }
 
   MachineDomTree &getBase() {
-    if (!DT)
-      DT.reset(new MachineDomTree());
     applySplitCriticalEdges();
-    return *DT;
+    return *this;
   }
 
-  void getAnalysisUsage(AnalysisUsage &AU) const override;
-
   MachineBasicBlock *getRoot() const {
     applySplitCriticalEdges();
-    return DT->getRoot();
+    return Base::getRoot();
   }
 
   MachineDomTreeNode *getRootNode() const {
     applySplitCriticalEdges();
-    return DT->getRootNode();
+    return const_cast<MachineDomTreeNode *>(Base::getRootNode());
   }
 
-  bool runOnMachineFunction(MachineFunction &F) override;
-
   void calculate(MachineFunction &F);
 
   bool dominates(const MachineDomTreeNode *A,
                  const MachineDomTreeNode *B) const {
     applySplitCriticalEdges();
-    return DT->dominates(A, B);
+    return Base::dominates(A, B);
   }
 
   void getDescendants(MachineBasicBlock *A,
                       SmallVectorImpl<MachineBasicBlock *> &Result) {
     applySplitCriticalEdges();
-    DT->getDescendants(A, Result);
+    Base::getDescendants(A, Result);
   }
 
   bool dominates(const MachineBasicBlock *A, const MachineBasicBlock *B) const {
     applySplitCriticalEdges();
-    return DT->dominates(A, B);
+    return Base::dominates(A, B);
   }
 
   // dominates - Return true if A dominates B. This performs the
@@ -133,7 +149,8 @@ class MachineDominatorTree : public MachineFunctionPass {
   bool dominates(const MachineInstr *A, const MachineInstr *B) const {
     applySplitCriticalEdges();
     const MachineBasicBlock *BBA = A->getParent(), *BBB = B->getParent();
-    if (BBA != BBB) return DT->dominates(BBA, BBB);
+    if (BBA != BBB)
+      return Base::dominates(BBA, BBB);
 
     // Loop through the basic block until we find A or B.
     MachineBasicBlock::const_iterator I = BBA->begin();
@@ -146,13 +163,13 @@ class MachineDominatorTree : public MachineFunctionPass {
   bool properlyDominates(const MachineDomTreeNode *A,
                          const MachineDomTreeNode *B) const {
     applySplitCriticalEdges();
-    return DT->properlyDominates(A, B);
+    return Base::properlyDominates(A, B);
   }
 
   bool properlyDominates(const MachineBasicBlock *A,
                          const MachineBasicBlock *B) const {
     applySplitCriticalEdges();
-    return DT->properlyDominates(A, B);
+    return Base::properlyDominates(A, B);
   }
 
   /// findNearestCommonDominator - Find nearest common dominator basic block
@@ -160,12 +177,12 @@ class MachineDominatorTree : public MachineFunctionPass {
   MachineBasicBlock *findNearestCommonDominator(MachineBasicBlock *A,
                                                 MachineBasicBlock *B) {
     applySplitCriticalEdges();
-    return DT->findNearestCommonDominator(A, B);
+    return Base::findNearestCommonDominator(A, B);
   }
 
   MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
     applySplitCriticalEdges();
-    return DT->getNode(BB);
+    return Base::getNode(BB);
   }
 
   /// getNode - return the (Post)DominatorTree node for the specified basic
@@ -173,7 +190,7 @@ class MachineDominatorTree : public MachineFunctionPass {
   ///
   MachineDomTreeNode *getNode(MachineBasicBlock *BB) const {
     applySplitCriticalEdges();
-    return DT->getNode(BB);
+    return Base::getNode(BB);
   }
 
   /// addNewBlock - Add a new node to the dominator tree information.  This
@@ -182,7 +199,7 @@ class MachineDominatorTree : public MachineFunctionPass {
   MachineDomTreeNode *addNewBlock(MachineBasicBlock *BB,
                                   MachineBasicBlock *DomBB) {
     applySplitCriticalEdges();
-    return DT->addNewBlock(BB, DomBB);
+    return Base::addNewBlock(BB, DomBB);
   }
 
   /// changeImmediateDominator - This method is used to update the dominator
@@ -191,13 +208,13 @@ class MachineDominatorTree : public MachineFunctionPass {
   void changeImmediateDominator(MachineBasicBlock *N,
                                 MachineBasicBlock *NewIDom) {
     applySplitCriticalEdges();
-    DT->changeImmediateDominator(N, NewIDom);
+    Base::changeImmediateDominator(N, NewIDom);
   }
 
   void changeImmediateDominator(MachineDomTreeNode *N,
                                 MachineDomTreeNode *NewIDom) {
     applySplitCriticalEdges();
-    DT->changeImmediateDominator(N, NewIDom);
+    Base::changeImmediateDominator(N, NewIDom);
   }
 
   /// eraseNode - Removes a node from  the dominator tree. Block must not
@@ -205,29 +222,23 @@ class MachineDominatorTree : public MachineFunctionPass {
   /// children list. Deletes dominator node associated with basic block BB.
   void eraseNode(MachineBasicBlock *BB) {
     applySplitCriticalEdges();
-    DT->eraseNode(BB);
+    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();
-    DT->splitBlock(NewBB);
+    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 DT->isReachableFromEntry(A);
+    return Base::isReachableFromEntry(A);
   }
 
-  void releaseMemory() override;
-
-  void verifyAnalysis() const override;
-
-  void print(raw_ostream &OS, const Module*) const override;
-
   /// Record that the critical edge (FromBB, ToBB) has been
   /// split with NewBB.
   /// This is best to use this method instead of directly update the
@@ -251,6 +262,31 @@ class MachineDominatorTree : public MachineFunctionPass {
   }
 };
 
+/// \brief Analysis pass which computes a \c MachineDominatorTree.
+class MachineDominatorTreeWrapperPass : public MachineFunctionPass {
+  MachineDominatorTree DT;
+
+public:
+  static char ID;
+
+  MachineDominatorTreeWrapperPass();
+
+  MachineDominatorTree &getDomTree() { return DT; }
+  const MachineDominatorTree &getDomTree() const { return DT; }
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+
+  void verifyAnalysis() const override;
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+  }
+
+  void releaseMemory() override;
+
+  void print(raw_ostream &OS, const Module *M = nullptr) const override;
+};
+
 //===-------------------------------------
 /// DominatorTree GraphTraits specialization so the DominatorTree can be
 /// iterable by generic graph iterators.
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index c4c1825bbf09e..585a34351c6b2 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -189,7 +189,7 @@ void initializeMachineCopyPropagationPass(PassRegistry&);
 void initializeMachineCycleInfoPrinterPassPass(PassRegistry &);
 void initializeMachineCycleInfoWrapperPassPass(PassRegistry &);
 void initializeMachineDominanceFrontierPass(PassRegistry&);
-void initializeMachineDominatorTreePass(PassRegistry&);
+void initializeMachineDominatorTreeWrapperPassPass(PassRegistry &);
 void initializeMachineFunctionPrinterPassPass(PassRegistry&);
 void initializeMachineFunctionSplitterPass(PassRegistry &);
 void initializeMachineLateInstrsCleanupPass(PassRegistry&);
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index e8bab26907b7e..e6f2a77f58af1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1728,7 +1728,8 @@ void AsmPrinter::emitFunctionBody() {
 
   if (isVerbose()) {
     // Get MachineDominatorTree or compute it on the fly if it's unavailable
-    MDT = getAnalysisIfAvailable<MachineDominatorTree>();
+    auto MDTWrapper = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
+    MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
     if (!MDT) {
       OwnedMDT = std::make_unique<MachineDominatorTree>();
       OwnedMDT->getBase().recalculate(*MF);
diff --git a/llvm/lib/CodeGen/CodeGen.cpp b/llvm/lib/CodeGen/CodeGen.cpp
index 544f1b7f59353..b9093208aad58 100644
--- a/llvm/lib/CodeGen/CodeGen.cpp
+++ b/llvm/lib/CodeGen/CodeGen.cpp
@@ -80,7 +80,7 @@ void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializeMachineCopyPropagationPass(Registry);
   initializeMachineCycleInfoPrinterPassPass(Registry);
   initializeMachineCycleInfoWrapperPassPass(Registry);
-  initializeMachineDominatorTreePass(Registry);
+  initializeMachineDominatorTreeWrapperPassPass(Registry);
   initializeMachineFunctionPrinterPassPass(Registry);
   initializeMachineLateInstrsCleanupPass(Registry);
   initializeMachineLICMPass(Registry);
diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp
index 2a7bee1618deb..30480e598acef 100644
--- a/llvm/lib/CodeGen/EarlyIfConversion.cpp
+++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp
@@ -790,15 +790,15 @@ char &llvm::EarlyIfConverterID = EarlyIfConverter::ID;
 INITIALIZE_PASS_BEGIN(EarlyIfConverter, DEBUG_TYPE,
                       "Early If Converter", false, false)
 INITIALIZE_PASS_DEPENDENCY(MachineBranchProbabilityInfo)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(MachineTraceMetrics)
 INITIALIZE_PASS_END(EarlyIfConverter, DEBUG_TYPE,
                     "Early If Converter", false, false)
 
 void EarlyIfConverter::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<MachineBranchProbabilityInfo>();
-  AU.addRequired<MachineDominatorTree>();
-  AU.addPreserved<MachineDominatorTree>();
+  AU.addRequired<MachineDominatorTreeWrapperPass>();
+  AU.addPreserved<MachineDominatorTreeWrapperPass>();
   AU.addRequired<MachineLoopInfo>();
   AU.addPreserved<MachineLoopInfo>();
   AU.addRequired<MachineTraceMetrics>();
@@ -1089,7 +1089,7 @@ bool EarlyIfConverter::runOnMachineFunction(MachineFunction &MF) {
   TRI = STI.getRegisterInfo();
   SchedModel = STI.getSchedModel();
   MRI = &MF.getRegInfo();
-  DomTree = &getAnalysis<MachineDominatorTree>();
+  DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
   Loops = &getAnalysis<MachineLoopInfo>();
   Traces = &getAnalysis<MachineTraceMetrics>();
   MinInstr = nullptr;
@@ -1144,15 +1144,15 @@ char &llvm::EarlyIfPredicatorID = EarlyIfPredicator::ID;
 
 INITIALIZE_PASS_BEGIN(EarlyIfPredicator, DEBUG_TYPE, "Early If Predicator",
                       false, false)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(MachineBranchProbabilityInfo)
 INITIALIZE_PASS_END(EarlyIfPredicator, DEBUG_TYPE, "Early If Predicator", false,
                     false)
 
 void EarlyIfPredicator::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<MachineBranchProbabilityInfo>();
-  AU.addRequired<MachineDominatorTree>();
-  AU.addPreserved<MachineDominatorTree>();
+  AU.addRequired<MachineDominatorTreeWrapperPass>();
+  AU.addPreserved<MachineDominatorTreeWrapperPass>();
   AU.addRequired<MachineLoopInfo>();
   AU.addPreserved<MachineLoopInfo>();
   MachineFunctionPass::getAnalysisUsage(AU);
@@ -1223,7 +1223,7 @@ bool EarlyIfPredicator::runOnMachineFunction(MachineFunction &MF) {
   TRI = STI.getRegisterInfo();
   MRI = &MF.getRegInfo();
   SchedModel.init(&STI);
-  DomTree = &getAnalysis<MachineDominatorTree>();
+  DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
   Loops = &getAnalysis<MachineLoopInfo>();
   MBPI = &getAnalysis<MachineBranchProbabilityInfo>();
 
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 69c671220db35..59ab5e0a610c3 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -135,8 +135,8 @@ class HoistSpillHelper : private LiveRangeEdit::Delegate {
                    VirtRegMap &vrm)
       : MF(mf), LIS(pass.getAnalysis<LiveIntervals>()),
         LSS(pass.getAnalysis<LiveStacks>()),
-        MDT(pass.getAnalysis<MachineDominatorTree>()), VRM(vrm),
-        MRI(mf.getRegInfo()), TII(*mf.getSubtarget().getInstrInfo()),
+        MDT(pass.getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree()),
+        VRM(vrm), MRI(mf.getRegInfo()), TII(*mf.getSubtarget().getInstrInfo()),
         TRI(*mf.getSubtarget().getRegisterInfo()),
         MBFI(pass.getAnalysis<MachineBlockFrequencyInfo>()),
         IPA(LIS, mf.getNumBlockIDs()) {}
@@ -192,8 +192,8 @@ class InlineSpiller : public Spiller {
                 VirtRegAuxInfo &VRAI)
       : MF(MF), LIS(Pass.getAnalysis<LiveIntervals>()),
         LSS(Pass.getAnalysis<LiveStacks>()),
-        MDT(Pass.getAnalysis<MachineDominatorTree>()), VRM(VRM),
-        MRI(MF.getRegInfo()), TII(*MF.getSubtarget().getInstrInfo()),
+        MDT(Pass.getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree()),
+        VRM(VRM), MRI(MF.getRegInfo()), TII(*MF.getSubtarget().getInstrInfo()),
         TRI(*MF.getSubtarget().getRegisterInfo()),
         MBFI(Pass.getAnalysis<MachineBlockFrequencyInfo>()),
         HSpiller(Pass, MF, VRM), VRAI(VRAI) {}
diff --git a/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp b/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
index 39b44b917d9e3..721b75900c8ef 100644
--- a/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
+++ b/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
@@ -64,7 +64,8 @@ LazyMachineBlockFrequencyInfoPass::calculateIfNotAvailable() const {
 
   auto &MBPI = getAnalysis<MachineBranchProbabilityInfo>();
   auto *MLI = getAnalysisIfAvailable<MachineLoopInfo>();
-  auto *MDT = getAnalysisIfAvailable<MachineDominatorTree>();
+  auto *MDTWrapper = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
+  auto *MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
   LLVM_DEBUG(dbgs() << "Building MachineBlockFrequencyInfo on the fly\n");
   LLVM_DEBUG(if (MLI) dbgs() << "LoopInfo is available\n");
 
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 3a59ae7ab0664..16d8e916ce668 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -78,13 +78,13 @@ char LiveDebugVariables::ID = 0;
 
 INITIALIZE_PASS_BEGIN(LiveDebugVariables, DEBUG_TYPE,
                 "Debug Variable Analysis", false, false)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LiveIntervals)
 INITIALIZE_PASS_END(LiveDebugVariables, DEBUG_TYPE,
                 "Debug Variable Analysis", false, false)
 
 void LiveDebugVariables::getAnalysisUsage(AnalysisUsage &AU) const {
-  AU.addRequired<MachineDominatorTree>();
+  AU.addRequired<MachineDominatorTreeWrapperPass>();
   AU.addRequiredTransitive<LiveIntervals>();
   AU.setPreservesAll();
   MachineFunctionPass::getAnalysisUsage(AU);
diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp
index 42c769399a140..f9162b444e03d 100644
--- a/llvm/lib/CodeGen/LiveIntervals.cpp
+++ b/llvm/lib/CodeGen/LiveIntervals.cpp
@@ -61,7 +61,7 @@ char LiveIntervals::ID = 0;
 char &llvm::LiveIntervalsID = LiveIntervals::ID;
 INITIALIZE_PASS_BEGIN(LiveIntervals, "liveintervals", "Live Interval Analysis",
                       false, false)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(SlotIndexes)
 INITIALIZE_PASS_END(LiveIntervals, "liveintervals",
                 "Live Interval Analysis", false, false)
@@ -123,7 +123,7 @@ bool LiveIntervals::runOnMachineFunction(MachineFunction &fn) {
   TRI = MF->getSubtarget().getRegisterInfo();
   TII = MF->getSubtarget().getInstrInfo();
   Indexes = &getAnalysis<SlotIndexes>();
-  DomTree = &getAnalysis<MachineDominatorTree>();
+  DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
 
   if (!LICalc)
     LICalc = new LiveIntervalCalc();
diff --git a/llvm/lib/CodeGen/MIRSampleProfile.cpp b/llvm/lib/CodeGen/MIRSampleProfile.cpp
index 6faa1ad1a7790..138cc56748762 100644
--- a/llvm/lib/CodeGen/MIRSampleProfile.cpp
+++ b/llvm/lib/CodeGen/MIRSampleProfile.cpp
@@ -70,7 +70,7 @@ INITIALIZE_PASS_BEGIN(MIRProfileLoaderPass, DEBUG_TYPE,
                       "Load MIR Sample Profile",
                       /* cfg = */ false, /* is_analysis = */ false)
 INITIALIZE_PASS_DEPENDENCY(MachineBlockFrequencyInfo)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(MachinePostDominatorTree)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
 INITIALIZE_PASS_DEPENDENCY(MachineOptimizationRemarkEmitterPass)
@@ -365,7 +365,7 @@ bool MIRProfileLoaderPass::runOnMachineFunction(MachineFunction &MF) {
                     << MF.getFunction().getName() << "\n");
   MBFI = &getAnalysis<MachineBlockFrequencyInfo>();
   MIRSampleLoader->setInitVals(
-      &getAnalysis<MachineDominatorTree>(),
+      &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree(),
       &getAnalysis<MachinePostDominatorTree>(), &getAnalysis<MachineLoopInfo>(),
       MBFI, &getAnalysis<MachineOptimizationRemarkEmitterPass>().getORE());
 
@@ -400,7 +400,7 @@ bool MIRProfileLoaderPass::doInitialization(Module &M) {
 void MIRPr...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 6, 2024

@llvm/pr-subscribers-backend-arm

Author: None (paperchalice)

Changes

Prepare for new pass manager version of MachineDominatorTreeAnalysis. We may need a machine dominator tree version of DomTreeUpdater to handle SplitCriticalEdge in some CodeGen passes.


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

88 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachineDominators.h (+75-39)
  • (modified) llvm/include/llvm/InitializePasses.h (+1-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+2-1)
  • (modified) llvm/lib/CodeGen/CodeGen.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/EarlyIfConversion.cpp (+8-8)
  • (modified) llvm/lib/CodeGen/InlineSpiller.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp (+2-1)
  • (modified) llvm/lib/CodeGen/LiveDebugVariables.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/LiveIntervals.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/MIRSampleProfile.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineCSE.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/MachineCombiner.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/MachineDominanceFrontier.cpp (+4-3)
  • (modified) llvm/lib/CodeGen/MachineDominators.cpp (+45-32)
  • (modified) llvm/lib/CodeGen/MachineLICM.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/MachineLoopInfo.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachinePipeliner.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineRegionInfo.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineScheduler.cpp (+5-5)
  • (modified) llvm/lib/CodeGen/MachineSink.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/MachineUniformityAnalysis.cpp (+4-3)
  • (modified) llvm/lib/CodeGen/PHIElimination.cpp (+3-3)
  • (modified) llvm/lib/CodeGen/PeepholeOptimizer.cpp (+5-4)
  • (modified) llvm/lib/CodeGen/PostRASchedulerList.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/PrologEpilogInserter.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/RegAllocBasic.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/RegAllocGreedy.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/RegAllocPBQP.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/ShrinkWrap.cpp (+4-4)
  • (modified) llvm/lib/CodeGen/UnreachableBlockElim.cpp (+4-2)
  • (modified) llvm/lib/CodeGen/XRayInstrumentation.cpp (+4-2)
  • (modified) llvm/lib/Target/AArch64/AArch64CleanupLocalDynamicTLSPass.cpp (+3-2)
  • (modified) llvm/lib/Target/AArch64/AArch64ConditionOptimizer.cpp (+4-4)
  • (modified) llvm/lib/Target/AArch64/AArch64ConditionalCompares.cpp (+4-4)
  • (modified) llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUGlobalISelDivergenceLowering.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPURegBankSelect.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/R600MachineCFGStructurizer.cpp (+3-3)
  • (modified) llvm/lib/Target/AMDGPU/R600OptimizeVectorRegisters.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/R600Packetizer.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp (+4-5)
  • (modified) llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp (+4-4)
  • (modified) llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SILowerI1Copies.cpp (+5-4)
  • (modified) llvm/lib/Target/AMDGPU/SIOptimizeVGPRLiveRange.cpp (+4-4)
  • (modified) llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp (+2-2)
  • (modified) llvm/lib/Target/ARC/ARCBranchFinalize.cpp (+1-1)
  • (modified) llvm/lib/Target/ARC/ARCOptAddrMode.cpp (+4-4)
  • (modified) llvm/lib/Target/ARM/ARMConstantIslandPass.cpp (+2-2)
  • (modified) llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (+4-4)
  • (modified) llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp (+5-4)
  • (modified) llvm/lib/Target/CSKY/CSKYConstantIslandPass.cpp (+1-1)
  • (modified) llvm/lib/Target/Hexagon/HexagonBitSimplify.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonConstExtenders.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonCopyHoisting.cpp (+2-2)
  • (modified) llvm/lib/Target/Hexagon/HexagonEarlyIfConv.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonGenInsert.cpp (+4-4)
  • (modified) llvm/lib/Target/Hexagon/HexagonGenMemAbsolute.cpp (+4-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonGenPredicate.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonHardwareLoops.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonOptAddrMode.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonRDFOpt.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp (+3-3)
  • (modified) llvm/lib/Target/Mips/MipsOptimizePICCall.cpp (+3-2)
  • (modified) llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/PowerPC/PPCBranchCoalescing.cpp (+3-3)
  • (modified) llvm/lib/Target/PowerPC/PPCCTRLoopsVerify.cpp (+3-3)
  • (modified) llvm/lib/Target/PowerPC/PPCMIPeephole.cpp (+4-4)
  • (modified) llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp (+2-2)
  • (modified) llvm/lib/Target/PowerPC/PPCVSXFMAMutate.cpp (+3-3)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVPostLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVPreLegalizerCombiner.cpp (+4-3)
  • (modified) llvm/lib/Target/SystemZ/SystemZLDCleanup.cpp (+3-2)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyMemIntrinsicResults.cpp (+3-3)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp (+3-3)
  • (modified) llvm/lib/Target/X86/X86FlagsCopyLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/X86/X86InstrInfo.cpp (+3-2)
  • (modified) llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp (+3-3)
diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 30c18ef410fab..d5231ad2ee3db 100644
--- a/llvm/include/llvm/CodeGen/MachineDominators.h
+++ b/llvm/include/llvm/CodeGen/MachineDominators.h
@@ -44,11 +44,37 @@ extern template class DominatorTreeBase<MachineBasicBlock, true>; // PostDomTree
 using MachineDomTree = DomTreeBase<MachineBasicBlock>;
 using MachineDomTreeNode = DomTreeNodeBase<MachineBasicBlock>;
 
+namespace DomTreeBuilder {
+using MBBDomTree = MachineDomTree;
+using MBBUpdates = ArrayRef<llvm::cfg::Update<MachineBasicBlock *>>;
+using MBBDomTreeGraphDiff = GraphDiff<MachineBasicBlock *, false>;
+
+extern template void Calculate<MBBDomTree>(MBBDomTree &DT);
+extern template void CalculateWithUpdates<MBBDomTree>(MBBDomTree &DT,
+                                                      MBBUpdates U);
+
+extern template void InsertEdge<MBBDomTree>(MBBDomTree &DT,
+                                            MachineBasicBlock *From,
+                                            MachineBasicBlock *To);
+
+extern template void DeleteEdge<MBBDomTree>(MBBDomTree &DT,
+                                            MachineBasicBlock *From,
+                                            MachineBasicBlock *To);
+
+extern template void ApplyUpdates<MBBDomTree>(MBBDomTree &DT,
+                                              MBBDomTreeGraphDiff &,
+                                              MBBDomTreeGraphDiff *);
+
+extern template bool Verify<MBBDomTree>(const MBBDomTree &DT,
+                                        MBBDomTree::VerificationLevel VL);
+} // namespace DomTreeBuilder
+
 //===-------------------------------------
 /// DominatorTree Class - Concrete subclass of DominatorTreeBase that is used to
 /// compute a normal dominator tree.
 ///
-class MachineDominatorTree : public MachineFunctionPass {
+class MachineDominatorTree : public MachineDomTree {
+  friend class MachineDominatorTreeWrapperPass;
   /// Helper structure used to hold all the basic blocks
   /// involved in the split of a critical edge.
   struct CriticalEdge {
@@ -70,62 +96,52 @@ class MachineDominatorTree : public MachineFunctionPass {
   /// such as BB == elt.NewBB.
   mutable SmallSet<MachineBasicBlock *, 32> NewBBs;
 
-  /// The DominatorTreeBase that is used to compute a normal dominator tree.
-  std::unique_ptr<MachineDomTree> DT;
-
   /// 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:
-  static char ID; // Pass ID, replacement for typeid
+  using Base = MachineDomTree;
 
-  MachineDominatorTree();
-  explicit MachineDominatorTree(MachineFunction &MF) : MachineFunctionPass(ID) {
-    calculate(MF);
-  }
+  MachineDominatorTree() = default;
+  explicit MachineDominatorTree(MachineFunction &MF) { calculate(MF); }
 
   MachineDomTree &getBase() {
-    if (!DT)
-      DT.reset(new MachineDomTree());
     applySplitCriticalEdges();
-    return *DT;
+    return *this;
   }
 
-  void getAnalysisUsage(AnalysisUsage &AU) const override;
-
   MachineBasicBlock *getRoot() const {
     applySplitCriticalEdges();
-    return DT->getRoot();
+    return Base::getRoot();
   }
 
   MachineDomTreeNode *getRootNode() const {
     applySplitCriticalEdges();
-    return DT->getRootNode();
+    return const_cast<MachineDomTreeNode *>(Base::getRootNode());
   }
 
-  bool runOnMachineFunction(MachineFunction &F) override;
-
   void calculate(MachineFunction &F);
 
   bool dominates(const MachineDomTreeNode *A,
                  const MachineDomTreeNode *B) const {
     applySplitCriticalEdges();
-    return DT->dominates(A, B);
+    return Base::dominates(A, B);
   }
 
   void getDescendants(MachineBasicBlock *A,
                       SmallVectorImpl<MachineBasicBlock *> &Result) {
     applySplitCriticalEdges();
-    DT->getDescendants(A, Result);
+    Base::getDescendants(A, Result);
   }
 
   bool dominates(const MachineBasicBlock *A, const MachineBasicBlock *B) const {
     applySplitCriticalEdges();
-    return DT->dominates(A, B);
+    return Base::dominates(A, B);
   }
 
   // dominates - Return true if A dominates B. This performs the
@@ -133,7 +149,8 @@ class MachineDominatorTree : public MachineFunctionPass {
   bool dominates(const MachineInstr *A, const MachineInstr *B) const {
     applySplitCriticalEdges();
     const MachineBasicBlock *BBA = A->getParent(), *BBB = B->getParent();
-    if (BBA != BBB) return DT->dominates(BBA, BBB);
+    if (BBA != BBB)
+      return Base::dominates(BBA, BBB);
 
     // Loop through the basic block until we find A or B.
     MachineBasicBlock::const_iterator I = BBA->begin();
@@ -146,13 +163,13 @@ class MachineDominatorTree : public MachineFunctionPass {
   bool properlyDominates(const MachineDomTreeNode *A,
                          const MachineDomTreeNode *B) const {
     applySplitCriticalEdges();
-    return DT->properlyDominates(A, B);
+    return Base::properlyDominates(A, B);
   }
 
   bool properlyDominates(const MachineBasicBlock *A,
                          const MachineBasicBlock *B) const {
     applySplitCriticalEdges();
-    return DT->properlyDominates(A, B);
+    return Base::properlyDominates(A, B);
   }
 
   /// findNearestCommonDominator - Find nearest common dominator basic block
@@ -160,12 +177,12 @@ class MachineDominatorTree : public MachineFunctionPass {
   MachineBasicBlock *findNearestCommonDominator(MachineBasicBlock *A,
                                                 MachineBasicBlock *B) {
     applySplitCriticalEdges();
-    return DT->findNearestCommonDominator(A, B);
+    return Base::findNearestCommonDominator(A, B);
   }
 
   MachineDomTreeNode *operator[](MachineBasicBlock *BB) const {
     applySplitCriticalEdges();
-    return DT->getNode(BB);
+    return Base::getNode(BB);
   }
 
   /// getNode - return the (Post)DominatorTree node for the specified basic
@@ -173,7 +190,7 @@ class MachineDominatorTree : public MachineFunctionPass {
   ///
   MachineDomTreeNode *getNode(MachineBasicBlock *BB) const {
     applySplitCriticalEdges();
-    return DT->getNode(BB);
+    return Base::getNode(BB);
   }
 
   /// addNewBlock - Add a new node to the dominator tree information.  This
@@ -182,7 +199,7 @@ class MachineDominatorTree : public MachineFunctionPass {
   MachineDomTreeNode *addNewBlock(MachineBasicBlock *BB,
                                   MachineBasicBlock *DomBB) {
     applySplitCriticalEdges();
-    return DT->addNewBlock(BB, DomBB);
+    return Base::addNewBlock(BB, DomBB);
   }
 
   /// changeImmediateDominator - This method is used to update the dominator
@@ -191,13 +208,13 @@ class MachineDominatorTree : public MachineFunctionPass {
   void changeImmediateDominator(MachineBasicBlock *N,
                                 MachineBasicBlock *NewIDom) {
     applySplitCriticalEdges();
-    DT->changeImmediateDominator(N, NewIDom);
+    Base::changeImmediateDominator(N, NewIDom);
   }
 
   void changeImmediateDominator(MachineDomTreeNode *N,
                                 MachineDomTreeNode *NewIDom) {
     applySplitCriticalEdges();
-    DT->changeImmediateDominator(N, NewIDom);
+    Base::changeImmediateDominator(N, NewIDom);
   }
 
   /// eraseNode - Removes a node from  the dominator tree. Block must not
@@ -205,29 +222,23 @@ class MachineDominatorTree : public MachineFunctionPass {
   /// children list. Deletes dominator node associated with basic block BB.
   void eraseNode(MachineBasicBlock *BB) {
     applySplitCriticalEdges();
-    DT->eraseNode(BB);
+    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();
-    DT->splitBlock(NewBB);
+    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 DT->isReachableFromEntry(A);
+    return Base::isReachableFromEntry(A);
   }
 
-  void releaseMemory() override;
-
-  void verifyAnalysis() const override;
-
-  void print(raw_ostream &OS, const Module*) const override;
-
   /// Record that the critical edge (FromBB, ToBB) has been
   /// split with NewBB.
   /// This is best to use this method instead of directly update the
@@ -251,6 +262,31 @@ class MachineDominatorTree : public MachineFunctionPass {
   }
 };
 
+/// \brief Analysis pass which computes a \c MachineDominatorTree.
+class MachineDominatorTreeWrapperPass : public MachineFunctionPass {
+  MachineDominatorTree DT;
+
+public:
+  static char ID;
+
+  MachineDominatorTreeWrapperPass();
+
+  MachineDominatorTree &getDomTree() { return DT; }
+  const MachineDominatorTree &getDomTree() const { return DT; }
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+
+  void verifyAnalysis() const override;
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+  }
+
+  void releaseMemory() override;
+
+  void print(raw_ostream &OS, const Module *M = nullptr) const override;
+};
+
 //===-------------------------------------
 /// DominatorTree GraphTraits specialization so the DominatorTree can be
 /// iterable by generic graph iterators.
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index c4c1825bbf09e..585a34351c6b2 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -189,7 +189,7 @@ void initializeMachineCopyPropagationPass(PassRegistry&);
 void initializeMachineCycleInfoPrinterPassPass(PassRegistry &);
 void initializeMachineCycleInfoWrapperPassPass(PassRegistry &);
 void initializeMachineDominanceFrontierPass(PassRegistry&);
-void initializeMachineDominatorTreePass(PassRegistry&);
+void initializeMachineDominatorTreeWrapperPassPass(PassRegistry &);
 void initializeMachineFunctionPrinterPassPass(PassRegistry&);
 void initializeMachineFunctionSplitterPass(PassRegistry &);
 void initializeMachineLateInstrsCleanupPass(PassRegistry&);
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index e8bab26907b7e..e6f2a77f58af1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1728,7 +1728,8 @@ void AsmPrinter::emitFunctionBody() {
 
   if (isVerbose()) {
     // Get MachineDominatorTree or compute it on the fly if it's unavailable
-    MDT = getAnalysisIfAvailable<MachineDominatorTree>();
+    auto MDTWrapper = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
+    MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
     if (!MDT) {
       OwnedMDT = std::make_unique<MachineDominatorTree>();
       OwnedMDT->getBase().recalculate(*MF);
diff --git a/llvm/lib/CodeGen/CodeGen.cpp b/llvm/lib/CodeGen/CodeGen.cpp
index 544f1b7f59353..b9093208aad58 100644
--- a/llvm/lib/CodeGen/CodeGen.cpp
+++ b/llvm/lib/CodeGen/CodeGen.cpp
@@ -80,7 +80,7 @@ void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializeMachineCopyPropagationPass(Registry);
   initializeMachineCycleInfoPrinterPassPass(Registry);
   initializeMachineCycleInfoWrapperPassPass(Registry);
-  initializeMachineDominatorTreePass(Registry);
+  initializeMachineDominatorTreeWrapperPassPass(Registry);
   initializeMachineFunctionPrinterPassPass(Registry);
   initializeMachineLateInstrsCleanupPass(Registry);
   initializeMachineLICMPass(Registry);
diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp
index 2a7bee1618deb..30480e598acef 100644
--- a/llvm/lib/CodeGen/EarlyIfConversion.cpp
+++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp
@@ -790,15 +790,15 @@ char &llvm::EarlyIfConverterID = EarlyIfConverter::ID;
 INITIALIZE_PASS_BEGIN(EarlyIfConverter, DEBUG_TYPE,
                       "Early If Converter", false, false)
 INITIALIZE_PASS_DEPENDENCY(MachineBranchProbabilityInfo)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(MachineTraceMetrics)
 INITIALIZE_PASS_END(EarlyIfConverter, DEBUG_TYPE,
                     "Early If Converter", false, false)
 
 void EarlyIfConverter::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<MachineBranchProbabilityInfo>();
-  AU.addRequired<MachineDominatorTree>();
-  AU.addPreserved<MachineDominatorTree>();
+  AU.addRequired<MachineDominatorTreeWrapperPass>();
+  AU.addPreserved<MachineDominatorTreeWrapperPass>();
   AU.addRequired<MachineLoopInfo>();
   AU.addPreserved<MachineLoopInfo>();
   AU.addRequired<MachineTraceMetrics>();
@@ -1089,7 +1089,7 @@ bool EarlyIfConverter::runOnMachineFunction(MachineFunction &MF) {
   TRI = STI.getRegisterInfo();
   SchedModel = STI.getSchedModel();
   MRI = &MF.getRegInfo();
-  DomTree = &getAnalysis<MachineDominatorTree>();
+  DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
   Loops = &getAnalysis<MachineLoopInfo>();
   Traces = &getAnalysis<MachineTraceMetrics>();
   MinInstr = nullptr;
@@ -1144,15 +1144,15 @@ char &llvm::EarlyIfPredicatorID = EarlyIfPredicator::ID;
 
 INITIALIZE_PASS_BEGIN(EarlyIfPredicator, DEBUG_TYPE, "Early If Predicator",
                       false, false)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(MachineBranchProbabilityInfo)
 INITIALIZE_PASS_END(EarlyIfPredicator, DEBUG_TYPE, "Early If Predicator", false,
                     false)
 
 void EarlyIfPredicator::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<MachineBranchProbabilityInfo>();
-  AU.addRequired<MachineDominatorTree>();
-  AU.addPreserved<MachineDominatorTree>();
+  AU.addRequired<MachineDominatorTreeWrapperPass>();
+  AU.addPreserved<MachineDominatorTreeWrapperPass>();
   AU.addRequired<MachineLoopInfo>();
   AU.addPreserved<MachineLoopInfo>();
   MachineFunctionPass::getAnalysisUsage(AU);
@@ -1223,7 +1223,7 @@ bool EarlyIfPredicator::runOnMachineFunction(MachineFunction &MF) {
   TRI = STI.getRegisterInfo();
   MRI = &MF.getRegInfo();
   SchedModel.init(&STI);
-  DomTree = &getAnalysis<MachineDominatorTree>();
+  DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
   Loops = &getAnalysis<MachineLoopInfo>();
   MBPI = &getAnalysis<MachineBranchProbabilityInfo>();
 
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 69c671220db35..59ab5e0a610c3 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -135,8 +135,8 @@ class HoistSpillHelper : private LiveRangeEdit::Delegate {
                    VirtRegMap &vrm)
       : MF(mf), LIS(pass.getAnalysis<LiveIntervals>()),
         LSS(pass.getAnalysis<LiveStacks>()),
-        MDT(pass.getAnalysis<MachineDominatorTree>()), VRM(vrm),
-        MRI(mf.getRegInfo()), TII(*mf.getSubtarget().getInstrInfo()),
+        MDT(pass.getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree()),
+        VRM(vrm), MRI(mf.getRegInfo()), TII(*mf.getSubtarget().getInstrInfo()),
         TRI(*mf.getSubtarget().getRegisterInfo()),
         MBFI(pass.getAnalysis<MachineBlockFrequencyInfo>()),
         IPA(LIS, mf.getNumBlockIDs()) {}
@@ -192,8 +192,8 @@ class InlineSpiller : public Spiller {
                 VirtRegAuxInfo &VRAI)
       : MF(MF), LIS(Pass.getAnalysis<LiveIntervals>()),
         LSS(Pass.getAnalysis<LiveStacks>()),
-        MDT(Pass.getAnalysis<MachineDominatorTree>()), VRM(VRM),
-        MRI(MF.getRegInfo()), TII(*MF.getSubtarget().getInstrInfo()),
+        MDT(Pass.getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree()),
+        VRM(VRM), MRI(MF.getRegInfo()), TII(*MF.getSubtarget().getInstrInfo()),
         TRI(*MF.getSubtarget().getRegisterInfo()),
         MBFI(Pass.getAnalysis<MachineBlockFrequencyInfo>()),
         HSpiller(Pass, MF, VRM), VRAI(VRAI) {}
diff --git a/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp b/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
index 39b44b917d9e3..721b75900c8ef 100644
--- a/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
+++ b/llvm/lib/CodeGen/LazyMachineBlockFrequencyInfo.cpp
@@ -64,7 +64,8 @@ LazyMachineBlockFrequencyInfoPass::calculateIfNotAvailable() const {
 
   auto &MBPI = getAnalysis<MachineBranchProbabilityInfo>();
   auto *MLI = getAnalysisIfAvailable<MachineLoopInfo>();
-  auto *MDT = getAnalysisIfAvailable<MachineDominatorTree>();
+  auto *MDTWrapper = getAnalysisIfAvailable<MachineDominatorTreeWrapperPass>();
+  auto *MDT = MDTWrapper ? &MDTWrapper->getDomTree() : nullptr;
   LLVM_DEBUG(dbgs() << "Building MachineBlockFrequencyInfo on the fly\n");
   LLVM_DEBUG(if (MLI) dbgs() << "LoopInfo is available\n");
 
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 3a59ae7ab0664..16d8e916ce668 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -78,13 +78,13 @@ char LiveDebugVariables::ID = 0;
 
 INITIALIZE_PASS_BEGIN(LiveDebugVariables, DEBUG_TYPE,
                 "Debug Variable Analysis", false, false)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LiveIntervals)
 INITIALIZE_PASS_END(LiveDebugVariables, DEBUG_TYPE,
                 "Debug Variable Analysis", false, false)
 
 void LiveDebugVariables::getAnalysisUsage(AnalysisUsage &AU) const {
-  AU.addRequired<MachineDominatorTree>();
+  AU.addRequired<MachineDominatorTreeWrapperPass>();
   AU.addRequiredTransitive<LiveIntervals>();
   AU.setPreservesAll();
   MachineFunctionPass::getAnalysisUsage(AU);
diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp
index 42c769399a140..f9162b444e03d 100644
--- a/llvm/lib/CodeGen/LiveIntervals.cpp
+++ b/llvm/lib/CodeGen/LiveIntervals.cpp
@@ -61,7 +61,7 @@ char LiveIntervals::ID = 0;
 char &llvm::LiveIntervalsID = LiveIntervals::ID;
 INITIALIZE_PASS_BEGIN(LiveIntervals, "liveintervals", "Live Interval Analysis",
                       false, false)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(SlotIndexes)
 INITIALIZE_PASS_END(LiveIntervals, "liveintervals",
                 "Live Interval Analysis", false, false)
@@ -123,7 +123,7 @@ bool LiveIntervals::runOnMachineFunction(MachineFunction &fn) {
   TRI = MF->getSubtarget().getRegisterInfo();
   TII = MF->getSubtarget().getInstrInfo();
   Indexes = &getAnalysis<SlotIndexes>();
-  DomTree = &getAnalysis<MachineDominatorTree>();
+  DomTree = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
 
   if (!LICalc)
     LICalc = new LiveIntervalCalc();
diff --git a/llvm/lib/CodeGen/MIRSampleProfile.cpp b/llvm/lib/CodeGen/MIRSampleProfile.cpp
index 6faa1ad1a7790..138cc56748762 100644
--- a/llvm/lib/CodeGen/MIRSampleProfile.cpp
+++ b/llvm/lib/CodeGen/MIRSampleProfile.cpp
@@ -70,7 +70,7 @@ INITIALIZE_PASS_BEGIN(MIRProfileLoaderPass, DEBUG_TYPE,
                       "Load MIR Sample Profile",
                       /* cfg = */ false, /* is_analysis = */ false)
 INITIALIZE_PASS_DEPENDENCY(MachineBlockFrequencyInfo)
-INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MachineDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(MachinePostDominatorTree)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
 INITIALIZE_PASS_DEPENDENCY(MachineOptimizationRemarkEmitterPass)
@@ -365,7 +365,7 @@ bool MIRProfileLoaderPass::runOnMachineFunction(MachineFunction &MF) {
                     << MF.getFunction().getName() << "\n");
   MBFI = &getAnalysis<MachineBlockFrequencyInfo>();
   MIRSampleLoader->setInitVals(
-      &getAnalysis<MachineDominatorTree>(),
+      &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree(),
       &getAnalysis<MachinePostDominatorTree>(), &getAnalysis<MachineLoopInfo>(),
       MBFI, &getAnalysis<MachineOptimizationRemarkEmitterPass>().getORE());
 
@@ -400,7 +400,7 @@ bool MIRProfileLoaderPass::doInitialization(Module &M) {
 void MIRPr...
[truncated]

@paperchalice paperchalice force-pushed the mdom-split branch 6 times, most recently from e2e9565 to 72d25b4 Compare June 6, 2024 09:24
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need a machine dominator tree version of DomTreeUpdater to handle SplitCriticalEdge in some CodeGen passes.

I'm surprised this doesn't already exist

@@ -44,11 +44,37 @@ extern template class DominatorTreeBase<MachineBasicBlock, true>; // PostDomTree
using MachineDomTree = DomTreeBase<MachineBasicBlock>;
using MachineDomTreeNode = DomTreeNodeBase<MachineBasicBlock>;

namespace DomTreeBuilder {
using MBBDomTree = MachineDomTree;
using MBBUpdates = ArrayRef<llvm::cfg::Update<MachineBasicBlock *>>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need llvm::?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from llvm/IR/Dominators.h, and unqualified name lookup can find the proper declaration.

//===-------------------------------------
/// DominatorTree Class - Concrete subclass of DominatorTreeBase that is used to
/// compute a normal dominator tree.
///
class MachineDominatorTree : public MachineFunctionPass {
class MachineDominatorTree : public MachineDomTree {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class + parent class naming is sort of confusing. I see MachineDomTree is just a typedef, so is there much point in keeping it? I think it would be clearer to make this directly : public DomTreeBase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MachineUniformityAnalysis and MachineVerifier use the MachineDomTree typedef, will try to change them to MachineDominatorTree.

…s result

Prepare for new pass manager version of `MachineDominatorTreeAnalysis`. We may need a machine dominator tree version of `DomTreeUpdater` to handle `SplitCriticalEdge` in some CodeGen passes.
@@ -266,6 +266,7 @@ class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
/// \brief Analysis pass which computes a \c MachineDominatorTree.
class MachineDominatorTreeWrapperPass : public MachineFunctionPass {
MachineDominatorTree DT;
bool IsDomTreeEmpty = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a pass preserves machine dominator tree analysis and machine dominator tree analysis result doesn't exists, verifyAnalysis will verify an empty dominator tree, which will trigger assertion. In old implementation it is a unique_ptr and can be nullptr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either make it std::optional or fix the verifier to not assert on the empty case

@@ -136,15 +144,19 @@ void MachineDominatorTree::applySplitCriticalEdges() const {

// Now, update DT with the collected dominance properties info.
Idx = 0;
dbgs() << "critical Edge to split: " << CriticalEdgesToSplit.size();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray debug printing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, debug code to resolve test failure due to empty dominator tree...

void MachineDominatorTreeWrapperPass::releaseMemory() { DT.reset(); }

void MachineDominatorTreeWrapperPass::verifyAnalysis() const {
if (VerifyMachineDomInfo && DT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think something is off here with DT not always being available. I tried running check-llvm with the current null check removed and only a few tests fail:

  LLVM :: CodeGen/Generic/available_externally_alias.ll
  LLVM :: CodeGen/Generic/externally_available.ll
  LLVM :: CodeGen/LoongArch/machinelicm-address-pseudos.ll
  LLVM :: CodeGen/LoongArch/opt-pipeline.ll
  LLVM :: CodeGen/PowerPC/aix-available-externally-linkage-fun.ll
  LLVM :: CodeGen/PowerPC/available-externally.ll
  LLVM :: CodeGen/X86/2009-05-23-available_externally.ll
  LLVM :: CodeGen/X86/dllimport-x86_64.ll
  LLVM :: CodeGen/X86/dllimport.ll
  LLVM :: CodeGen/X86/hidden-vis-pic.ll
  LLVM :: CodeGen/X86/partially-coalesced3.mir
  LLVM :: CodeGen/X86/visibility.ll
  LLVM :: CodeGen/X86/win32-eh-available-externally.ll

so I think this is probably just a bug somewhere?

In any case, that's a pre-existing issue not related to this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that a function F which hasAvailableExternallyLinkage is true and a pass P requires MachineDominatorTreeWrapperPass and preserves it.

bool MachineFunctionPass::runOnFunction(Function &F) {
// Do not codegen any 'available_externally' functions at all, they have
// definitions outside the translation unit.
if (F.hasAvailableExternallyLinkage())
return false;

Then the construction of DT in MachineDominatorTreeWrapperPass is skipped.

Legacy pass manager will still try to verify the result which is not exist:

AnalysisUsage *AnUsage = TPM->findAnalysisUsage(P);
const AnalysisUsage::VectorType &PreservedSet = AnUsage->getPreservedSet();
// Verify preserved analysis
for (AnalysisID AID : PreservedSet) {
if (Pass *AP = findAnalysisPass(AID, true)) {
TimeRegion PassTimer(getPassTimer(AP));
AP->verifyAnalysis();
}
}
}

I think this can explain why externally_available.ll-like tests are failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid running the passes on the won't be emitted functions? This feels like a clumsy way of skipping them

Copy link
Contributor Author

@paperchalice paperchalice Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MachineFunctionPass is subclass of FunctionPass, maybe we can just change F.isDeclaration() -> F.isDeclaration() || F.hasAvailableExternallyLinkage()

bool FPPassManager::runOnFunction(Function &F) {
if (F.isDeclaration())
return false;

Not sure what the impact will be, but CodeGen pipeline is the only user of legacy pass manager we could try it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe instead change MachineFunctionPass::runOnFunction() to skip declarations?

@paperchalice paperchalice merged commit 837dc54 into llvm:main Jun 11, 2024
7 checks passed
paperchalice added a commit to paperchalice/llvm-project that referenced this pull request Jun 11, 2024
paperchalice added a commit to paperchalice/llvm-project that referenced this pull request Jun 11, 2024
paperchalice added a commit that referenced this pull request Jun 12, 2024
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants