[llvm] c60149b - Revert "[Dominator] Add findNearestCommonDominator() for Instructions (NFC)"

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-01-06T17:36:01+01:00
New Revision: c60149b49e18a4cedc4c57d3bcb42d95061245f1

URL: https://github.com/llvm/llvm-project/commit/c60149b49e18a4cedc4c57d3bcb42d95061245f1
DIFF: https://github.com/llvm/llvm-project/commit/c60149b49e18a4cedc4c57d3bcb42d95061245f1.diff

LOG: Revert "[Dominator] Add findNearestCommonDominator() for Instructions (NFC)"

This reverts commit 7f0de9573f758f5f9108795850337a5acbd17eef.

This is missing handling for !isReachableFromEntry() blocks, which
may be relevant for some callers. Revert for now.

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Dominators.h b/llvm/include/llvm/IR/Dominators.h
index c2d080bc2004..7fbc7608dea4 100644
--- a/llvm/include/llvm/IR/Dominators.h
+++ b/llvm/include/llvm/IR/Dominators.h
@@ -219,14 +219,6 @@ 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();

diff  --git a/llvm/lib/Analysis/CaptureTracking.cpp b/llvm/lib/Analysis/CaptureTracking.cpp
index 7f3a2b49aca9..f4fd660ac7e0 100644
--- a/llvm/lib/Analysis/CaptureTracking.cpp
+++ b/llvm/lib/Analysis/CaptureTracking.cpp
@@ -179,10 +179,25 @@ namespace {
       if (EphValues.contains(I))
         return false;
 
-      if (!EarliestCapture)
+      if (!EarliestCapture) {
         EarliestCapture = I;
-      else
-        EarliestCapture = DT.findNearestCommonDominator(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();
+        }
+      }
       Captured = true;
 
       // Return false to continue analysis; we need to see all potential

diff  --git a/llvm/lib/IR/Dominators.cpp b/llvm/lib/IR/Dominators.cpp
index 6693480189fd..09be2a8ef605 100644
--- a/llvm/lib/IR/Dominators.cpp
+++ b/llvm/lib/IR/Dominators.cpp
@@ -355,20 +355,6 @@ 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;
-  BasicBlock *DomBB = findNearestCommonDominator(BB1, BB2);
-  if (BB1 == DomBB)
-    return I1;
-  if (BB2 == DomBB)
-    return I2;
-  return DomBB->getTerminator();
-}
-
 //===----------------------------------------------------------------------===//
 //  DominatorTreeAnalysis and related pass implementations
 //===----------------------------------------------------------------------===//

diff  --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 7325694eec07..e60382bc933d 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -2557,8 +2557,15 @@ 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)
-    IVIncInsertPos = DT.findNearestCommonDominator(IVIncInsertPos, Inst);
+  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();
+  }
 }
 
 /// Determine if the given use can accommodate a fixup at the given offset and

diff  --git a/llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp b/llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp
index 4ec7181ad859..59edc68684d7 100644
--- a/llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp
+++ b/llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp
@@ -187,7 +187,19 @@ Instruction *TLSVariableHoistPass::getDomInst(Instruction *I1,
                                               Instruction *I2) {
   if (!I1)
     return I2;
-  return DT->findNearestCommonDominator(I1, 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;
 }
 
 BasicBlock::iterator TLSVariableHoistPass::findInsertPos(Function &Fn,

diff  --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
index 4e83d2f6e3c6..a7fe065c989f 100644
--- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
@@ -106,8 +106,13 @@ static Instruction *findCommonDominator(ArrayRef<Instruction *> Instructions,
                                         DominatorTree &DT) {
   Instruction *CommonDom = nullptr;
   for (auto *Insn : Instructions)
-    CommonDom =
-        CommonDom ? DT.findNearestCommonDominator(CommonDom, Insn) : Insn;
+    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();
   assert(CommonDom && "Common dominator not found?");
   return CommonDom;
 }