[llvm] c608b49 - [SCEV] Tweak the algorithm for figuring out if flags must apply to a SCEV [mostly-NFC]

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-10-05T11:20:48-07:00
New Revision: c608b49d67e0c22cc3537569f76af500097cd3b4

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

LOG: [SCEV] Tweak the algorithm for figuring out if flags must apply to a SCEV [mostly-NFC]

Behavior wise, this patch should be mostly NFC.  The only behavior difference known is that on the isSCEVExprNeverPoison path we'll consider a bound imposed by the SCEVable operands (if any).

Algorithmically, it's an invert of the existing code.  Previously, we checked for each operand if we could find a bound, then checked for must-execute given that bound.  With the patch, we use dominance to refine the innermost bound, then check must execute once.  The interesting case is when we have multiple unknowns within a single basic block.  While both dominance and must-execute are worst-case linear walks within the block, only dominance is cached.  As such, refining based on dominance should be more efficient.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/ScalarEvolution.h
    llvm/lib/Analysis/ScalarEvolution.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index c3ee802342a50..df1b7c2ad157d 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -1925,6 +1925,10 @@ class ScalarEvolution {
   /// (See scope definition rules associated with flag discussion above)
   const Instruction *getDefiningScopeBound(const SCEV *S);
 
+  /// Return a scope which provides an upper bound on the defining scope for
+  /// a SCEV with the operands in Ops.
+  const Instruction *getDefiningScopeBound(ArrayRef<const SCEV *> Ops);
+
   /// Given two instructions in the same function, return true if we can
   /// prove B must execute given A executes.
   bool isGuaranteedToTransferExecutionTo(const Instruction *A,

diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index bc698a1d9847b..c09180b2c197f 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -2802,11 +2802,10 @@ const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *> &Ops,
       // Proving that entry to the outer scope neccesitates entry to the inner
       // scope, thus proves the program undefined if the flags would be violated
       // in the outer scope.
-      const bool CanPropagateFlags = llvm::any_of(LIOps, [&](const SCEV *S) {
-        auto *ReachI = &*AddRecLoop->getHeader()->begin();
-        auto *DefI = getDefiningScopeBound(S);
-        return isGuaranteedToTransferExecutionTo(DefI, ReachI);
-      });
+      auto *DefI = getDefiningScopeBound(LIOps);
+      auto *ReachI = &*AddRecLoop->getHeader()->begin();
+      const bool CanPropagateFlags =
+        isGuaranteedToTransferExecutionTo(DefI, ReachI);
       auto AddFlags = CanPropagateFlags ? Flags : SCEV::FlagAnyWrap;
       AddRecOps[0] = getAddExpr(LIOps, AddFlags, Depth + 1);
 
@@ -6594,6 +6593,18 @@ const Instruction *ScalarEvolution::getDefiningScopeBound(const SCEV *S) {
   return &*F.getEntryBlock().begin();
 }
 
+const Instruction *
+ScalarEvolution::getDefiningScopeBound(ArrayRef<const SCEV *> Ops) {
+  const Instruction *Bound = &*F.getEntryBlock().begin();
+  for (auto *S : Ops) {
+    auto *DefI = getDefiningScopeBound(S);
+    if (DT.dominates(Bound, DefI))
+      Bound = DefI;
+  }
+  return Bound;
+}
+
+
 static bool
 isGuaranteedToTransferExecutionToSuccessor(BasicBlock::const_iterator Begin,
                                            BasicBlock::const_iterator End) {
@@ -6657,15 +6668,16 @@ bool ScalarEvolution::isSCEVExprNeverPoison(const Instruction *I) {
   // executed every time we enter that scope.  When the bounding scope is a
   // loop (the common case), this is equivalent to proving I executes on every
   // iteration of that loop.
+  SmallVector<const SCEV *> SCEVOps;
   for (const Use &Op : I->operands()) {
     // I could be an extractvalue from a call to an overflow intrinsic.
     // TODO: We can do better here in some cases.
-    if (!isSCEVable(Op->getType()))
-      return false;
-    auto *DefI = getDefiningScopeBound(getSCEV(Op));
-    if (isGuaranteedToTransferExecutionTo(DefI, I))
-      return true;
+    if (isSCEVable(Op->getType()))
+      SCEVOps.push_back(getSCEV(Op));
   }
+  auto *DefI = getDefiningScopeBound(SCEVOps);
+  if (isGuaranteedToTransferExecutionTo(DefI, I))
+    return true;
   return false;
 }