Reapply [Dominators] Add findNearestCommonDominator() for Instructions (NFC)
authorNikita Popov <npopov@redhat.com>
Fri, 6 Jan 2023 15:56:34 +0000 (16:56 +0100)
committerNikita Popov <npopov@redhat.com>
Tue, 10 Jan 2023 11:16:31 +0000 (12:16 +0100)
Reapply with checks for instructions in unreachable blocks. A test
case for this was added in 1ee4a93b15bb.

-----

This is a recurring pattern: We want to find the nearest common
dominator (instruction) for two instructions, but currently only
provide an API for the nearest common dominator of two basic blocks.

Add an overload that accepts and return instructions.

llvm/include/llvm/IR/Dominators.h
llvm/lib/Analysis/CaptureTracking.cpp
llvm/lib/IR/Dominators.cpp
llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp
llvm/lib/Transforms/Utils/SimplifyIndVar.cpp

index 7fbc760..c2d080b 100644 (file)
@@ -219,6 +219,14 @@ class DominatorTree : public DominatorTreeBase<BasicBlock, false> {
   /// Provide an overload for a Use.
   bool isReachableFromEntry(const Use &U) const;
 
+  // Ensure base class overloads are visible.
+  using Base::findNearestCommonDominator;
+
+  /// Find the nearest instruction I that dominates both I1 and I2, in the sense
+  /// that a result produced before I will be available at both I1 and I2.
+  Instruction *findNearestCommonDominator(Instruction *I1,
+                                          Instruction *I2) const;
+
   // Pop up a GraphViz/gv window with the Dominator Tree rendered using `dot`.
   void viewGraph(const Twine &Name, const Twine &Title);
   void viewGraph();
index f4fd660..7f3a2b4 100644 (file)
@@ -179,25 +179,10 @@ namespace {
       if (EphValues.contains(I))
         return false;
 
-      if (!EarliestCapture) {
+      if (!EarliestCapture)
         EarliestCapture = I;
-      } else if (EarliestCapture->getParent() == I->getParent()) {
-        if (I->comesBefore(EarliestCapture))
-          EarliestCapture = I;
-      } else {
-        BasicBlock *CurrentBB = I->getParent();
-        BasicBlock *EarliestBB = EarliestCapture->getParent();
-        if (DT.dominates(EarliestBB, CurrentBB)) {
-          // EarliestCapture already comes before the current use.
-        } else if (DT.dominates(CurrentBB, EarliestBB)) {
-          EarliestCapture = I;
-        } else {
-          // Otherwise find the nearest common dominator and use its terminator.
-          auto *NearestCommonDom =
-              DT.findNearestCommonDominator(CurrentBB, EarliestBB);
-          EarliestCapture = NearestCommonDom->getTerminator();
-        }
-      }
+      else
+        EarliestCapture = DT.findNearestCommonDominator(EarliestCapture, I);
       Captured = true;
 
       // Return false to continue analysis; we need to see all potential
index 09be2a8..7c620c3 100644 (file)
@@ -355,6 +355,24 @@ bool DominatorTree::dominates(const BasicBlockEdge &BBE1,
   return dominates(BBE1, BBE2.getStart());
 }
 
+Instruction *DominatorTree::findNearestCommonDominator(Instruction *I1,
+                                                       Instruction *I2) const {
+  BasicBlock *BB1 = I1->getParent();
+  BasicBlock *BB2 = I2->getParent();
+  if (BB1 == BB2)
+    return I1->comesBefore(I2) ? I1 : I2;
+  if (!isReachableFromEntry(BB2))
+    return I1;
+  if (!isReachableFromEntry(BB1))
+    return I2;
+  BasicBlock *DomBB = findNearestCommonDominator(BB1, BB2);
+  if (BB1 == DomBB)
+    return I1;
+  if (BB2 == DomBB)
+    return I2;
+  return DomBB->getTerminator();
+}
+
 //===----------------------------------------------------------------------===//
 //  DominatorTreeAnalysis and related pass implementations
 //===----------------------------------------------------------------------===//
index 7b8bce6..6285298 100644 (file)
@@ -2557,15 +2557,8 @@ LSRInstance::OptimizeLoopTermCond() {
   // must dominate all the post-inc comparisons we just set up, and it must
   // dominate the loop latch edge.
   IVIncInsertPos = L->getLoopLatch()->getTerminator();
-  for (Instruction *Inst : PostIncs) {
-    BasicBlock *BB =
-      DT.findNearestCommonDominator(IVIncInsertPos->getParent(),
-                                    Inst->getParent());
-    if (BB == Inst->getParent())
-      IVIncInsertPos = Inst;
-    else if (BB != IVIncInsertPos->getParent())
-      IVIncInsertPos = BB->getTerminator();
-  }
+  for (Instruction *Inst : PostIncs)
+    IVIncInsertPos = DT.findNearestCommonDominator(IVIncInsertPos, Inst);
 }
 
 /// Determine if the given use can accommodate a fixup at the given offset and
index 59edc68..4ec7181 100644 (file)
@@ -187,19 +187,7 @@ Instruction *TLSVariableHoistPass::getDomInst(Instruction *I1,
                                               Instruction *I2) {
   if (!I1)
     return I2;
-  if (DT->dominates(I1, I2))
-    return I1;
-  if (DT->dominates(I2, I1))
-    return I2;
-
-  // If there is no dominance relation, use common dominator.
-  BasicBlock *DomBB =
-      DT->findNearestCommonDominator(I1->getParent(), I2->getParent());
-
-  Instruction *Dom = DomBB->getTerminator();
-  assert(Dom && "Common dominator not found!");
-
-  return Dom;
+  return DT->findNearestCommonDominator(I1, I2);
 }
 
 BasicBlock::iterator TLSVariableHoistPass::findInsertPos(Function &Fn,
index a7fe065..4e83d2f 100644 (file)
@@ -106,13 +106,8 @@ static Instruction *findCommonDominator(ArrayRef<Instruction *> Instructions,
                                         DominatorTree &DT) {
   Instruction *CommonDom = nullptr;
   for (auto *Insn : Instructions)
-    if (!CommonDom || DT.dominates(Insn, CommonDom))
-      CommonDom = Insn;
-    else if (!DT.dominates(CommonDom, Insn))
-      // If there is no dominance relation, use common dominator.
-      CommonDom =
-          DT.findNearestCommonDominator(CommonDom->getParent(),
-                                        Insn->getParent())->getTerminator();
+    CommonDom =
+        CommonDom ? DT.findNearestCommonDominator(CommonDom, Insn) : Insn;
   assert(CommonDom && "Common dominator not found?");
   return CommonDom;
 }