Revert "[SCEV] Collect and merge loop guards through PHI nodes with multiple incoming values (llvm#113915)"

This reverts commit 7c8e05a.

Author: Florian Hahn

llvm/include/llvm/Analysis/ScalarEvolution.h

@@ -1326,7 +1326,7 @@ class ScalarEvolution {
     /// Collect loop guards in \p Guards, starting from PHINode \p
     /// Phi, by calling \p collectFromBlock on the incoming blocks of
     /// \Phi and trying to merge the found constraints into a single
-    /// combined one for \p Phi.
+    /// combined on for \p Phi.
     static void collectFromPHI(
         ScalarEvolution &SE, ScalarEvolution::LoopGuards &Guards,
         const PHINode &Phi, SmallPtrSetImpl<const BasicBlock *> &VisitedBlocks,

llvm/lib/Analysis/ScalarEvolution.cpp

@@ -224,10 +224,6 @@ static cl::opt<unsigned> RangeIterThreshold(
     cl::desc("Threshold for switching to iteratively computing SCEV ranges"),
     cl::init(32));
 
-static cl::opt<unsigned> MaxLoopGuardCollectionDepth(
-    "scalar-evolution-max-loop-guard-collection-depth", cl::Hidden,
-    cl::desc("Maximum depth for recursive loop guard collection"), cl::init(1));
-
 static cl::opt<bool>
 ClassifyExpressions("scalar-evolution-classify-expressions",
     cl::Hidden, cl::init(true),
@@ -10707,7 +10703,7 @@ ScalarEvolution::getPredecessorWithUniqueSuccessorForBB(const BasicBlock *BB)
   if (const Loop *L = LI.getLoopFor(BB))
     return {L->getLoopPredecessor(), L->getHeader()};
 
-  return {nullptr, BB};
+  return {nullptr, nullptr};
 }
 
 /// SCEV structural equivalence is usually sufficient for testing whether two
@@ -15325,83 +15321,7 @@ bool ScalarEvolution::matchURem(const SCEV *Expr, const SCEV *&LHS,
 
 ScalarEvolution::LoopGuards
 ScalarEvolution::LoopGuards::collect(const Loop *L, ScalarEvolution &SE) {
-  BasicBlock *Header = L->getHeader();
-  BasicBlock *Pred = L->getLoopPredecessor();
   LoopGuards Guards(SE);
-  if (!Pred)
-    return Guards;
-  SmallPtrSet<const BasicBlock *, 8> VisitedBlocks;
-  collectFromBlock(SE, Guards, Header, Pred, VisitedBlocks);
-  return Guards;
-}
-
-void ScalarEvolution::LoopGuards::collectFromPHI(
-    ScalarEvolution &SE, ScalarEvolution::LoopGuards &Guards,
-    const PHINode &Phi, SmallPtrSetImpl<const BasicBlock *> &VisitedBlocks,
-    SmallDenseMap<const BasicBlock *, LoopGuards> &IncomingGuards,
-    unsigned Depth) {
-  if (!SE.isSCEVable(Phi.getType()))
-    return;
-
-  using MinMaxPattern = std::pair<const SCEVConstant *, SCEVTypes>;
-  auto GetMinMaxConst = [&](unsigned IncomingIdx) -> MinMaxPattern {
-    const BasicBlock *InBlock = Phi.getIncomingBlock(IncomingIdx);
-    if (!VisitedBlocks.insert(InBlock).second)
-      return {nullptr, scCouldNotCompute};
-    auto [G, Inserted] = IncomingGuards.try_emplace(InBlock, LoopGuards(SE));
-    if (Inserted)
-      collectFromBlock(SE, G->second, Phi.getParent(), InBlock, VisitedBlocks,
-                       Depth + 1);
-    auto &RewriteMap = G->second.RewriteMap;
-    if (RewriteMap.empty())
-      return {nullptr, scCouldNotCompute};
-    auto S = RewriteMap.find(SE.getSCEV(Phi.getIncomingValue(IncomingIdx)));
-    if (S == RewriteMap.end())
-      return {nullptr, scCouldNotCompute};
-    auto *SM = dyn_cast_if_present<SCEVMinMaxExpr>(S->second);
-    if (!SM)
-      return {nullptr, scCouldNotCompute};
-    if (const SCEVConstant *C0 = dyn_cast<SCEVConstant>(SM->getOperand(0)))
-      return {C0, SM->getSCEVType()};
-    return {nullptr, scCouldNotCompute};
-  };
-  auto MergeMinMaxConst = [](MinMaxPattern P1,
-                             MinMaxPattern P2) -> MinMaxPattern {
-    auto [C1, T1] = P1;
-    auto [C2, T2] = P2;
-    if (!C1 || !C2 || T1 != T2)
-      return {nullptr, scCouldNotCompute};
-    switch (T1) {
-    case scUMaxExpr:
-      return {C1->getAPInt().ult(C2->getAPInt()) ? C1 : C2, T1};
-    case scSMaxExpr:
-      return {C1->getAPInt().slt(C2->getAPInt()) ? C1 : C2, T1};
-    case scUMinExpr:
-      return {C1->getAPInt().ugt(C2->getAPInt()) ? C1 : C2, T1};
-    case scSMinExpr:
-      return {C1->getAPInt().sgt(C2->getAPInt()) ? C1 : C2, T1};
-    default:
-      llvm_unreachable("Trying to merge non-MinMaxExpr SCEVs.");
-    }
-  };
-  auto P = GetMinMaxConst(0);
-  for (unsigned int In = 1; In < Phi.getNumIncomingValues(); In++) {
-    if (!P.first)
-      break;
-    P = MergeMinMaxConst(P, GetMinMaxConst(In));
-  }
-  if (P.first) {
-    const SCEV *LHS = SE.getSCEV(const_cast<PHINode *>(&Phi));
-    SmallVector<const SCEV *, 2> Ops({P.first, LHS});
-    const SCEV *RHS = SE.getMinMaxExpr(P.second, Ops);
-    Guards.RewriteMap.insert({LHS, RHS});
-  }
-}
-
-void ScalarEvolution::LoopGuards::collectFromBlock(
-    ScalarEvolution &SE, ScalarEvolution::LoopGuards &Guards,
-    const BasicBlock *Block, const BasicBlock *Pred,
-    SmallPtrSetImpl<const BasicBlock *> &VisitedBlocks, unsigned Depth) {
   SmallVector<const SCEV *> ExprsToRewrite;
   auto CollectCondition = [&](ICmpInst::Predicate Predicate, const SCEV *LHS,
                               const SCEV *RHS,
@@ -15735,13 +15655,14 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
     }
   };
 
+  BasicBlock *Header = L->getHeader();
   SmallVector<PointerIntPair<Value *, 1, bool>> Terms;
   // First, collect information from assumptions dominating the loop.
   for (auto &AssumeVH : SE.AC.assumptions()) {
     if (!AssumeVH)
       continue;
     auto *AssumeI = cast<CallInst>(AssumeVH);
-    if (!SE.DT.dominates(AssumeI, Block))
+    if (!SE.DT.dominates(AssumeI, Header))
       continue;
     Terms.emplace_back(AssumeI->getOperand(0), true);
   }
@@ -15752,45 +15673,27 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
   if (GuardDecl)
     for (const auto *GU : GuardDecl->users())
       if (const auto *Guard = dyn_cast<IntrinsicInst>(GU))
-        if (Guard->getFunction() == Block->getParent() &&
-            SE.DT.dominates(Guard, Block))
+        if (Guard->getFunction() == Header->getParent() &&
+            SE.DT.dominates(Guard, Header))
           Terms.emplace_back(Guard->getArgOperand(0), true);
 
   // Third, collect conditions from dominating branches. Starting at the loop
   // predecessor, climb up the predecessor chain, as long as there are
   // predecessors that can be found that have unique successors leading to the
   // original header.
   // TODO: share this logic with isLoopEntryGuardedByCond.
-  unsigned NumCollectedConditions = 0;
-  VisitedBlocks.insert(Block);
-  std::pair<const BasicBlock *, const BasicBlock *> Pair(Pred, Block);
-  for (; Pair.first;
+  for (std::pair<const BasicBlock *, const BasicBlock *> Pair(
+           L->getLoopPredecessor(), Header);
+       Pair.first;
        Pair = SE.getPredecessorWithUniqueSuccessorForBB(Pair.first)) {
-    VisitedBlocks.insert(Pair.second);
+
     const BranchInst *LoopEntryPredicate =
         dyn_cast<BranchInst>(Pair.first->getTerminator());
     if (!LoopEntryPredicate || LoopEntryPredicate->isUnconditional())
       continue;
 
     Terms.emplace_back(LoopEntryPredicate->getCondition(),
                        LoopEntryPredicate->getSuccessor(0) == Pair.second);
-    NumCollectedConditions++;
-
-    // If we are recursively collecting guards stop after 2
-    // conditions to limit compile-time impact for now.
-    if (Depth > 0 && NumCollectedConditions == 2)
-      break;
-  }
-  // Finally, if we stopped climbing the predecessor chain because
-  // there wasn't a unique one to continue, try to collect conditions
-  // for PHINodes by recursively following all of their incoming
-  // blocks and try to merge the found conditions to build a new one
-  // for the Phi.
-  if (Pair.second->hasNPredecessorsOrMore(2) &&
-      Depth < MaxLoopGuardCollectionDepth) {
-    SmallDenseMap<const BasicBlock *, LoopGuards> IncomingGuards;
-    for (auto &Phi : Pair.second->phis())
-      collectFromPHI(SE, Guards, Phi, VisitedBlocks, IncomingGuards, Depth);
   }
 
   // Now apply the information from the collected conditions to
@@ -15847,6 +15750,7 @@ void ScalarEvolution::LoopGuards::collectFromBlock(
       Guards.RewriteMap.insert({Expr, Guards.rewrite(RewriteTo)});
     }
   }
+  return Guards;
 }
 
 const SCEV *ScalarEvolution::LoopGuards::rewrite(const SCEV *Expr) const {