[PATCH] D104140: [SCEV] Allow negative steps for LT exit count computation

[Philip Reames via Phabricator via llvm-commits]

reames updated this revision to Diff 359947.
reames added a comment.

Address Eli's review comment which isn't covered by D106197 <https://reviews.llvm.org/D106197>.

I'm going to land some tests for this case and probably split this patch since this appears to be a flaw in the existing logic for potentially zero strides.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104140/new/

https://reviews.llvm.org/D104140

Files:
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/ScalarEvolution/trip-count-negative-stride.ll


Index: llvm/test/Analysis/ScalarEvolution/trip-count-negative-stride.ll
===================================================================
--- llvm/test/Analysis/ScalarEvolution/trip-count-negative-stride.ll
+++ llvm/test/Analysis/ScalarEvolution/trip-count-negative-stride.ll
@@ -85,8 +85,8 @@
 }
 
 ; CHECK-LABEL: Determining loop execution counts for: @ult_129_unknown_start
-; CHECK: Loop %for.body: Unpredictable backedge-taken count
-; CHECK: Loop %for.body: Unpredictable max backedge-taken count
+; CHECK: Loop %for.body: backedge-taken count is ((-1 + (-1 * %start) + (-128 umax (-127 + %start))) /u -127)
+; CHECK: Loop %for.body: max backedge-taken count is -128
 
 define void @ult_129_unknown_start(i8 %start) mustprogress {
 entry:
@@ -308,8 +308,8 @@
 }
 
 ; CHECK-LABEL: Determining loop execution counts for: @slt_129_unknown_start
-; CHECK: Loop %for.body: Unpredictable backedge-taken count
-; CHECK: Loop %for.body: Unpredictable max backedge-taken count
+; CHECK: Loop %for.body: backedge-taken count is ((-1 + (-1 * %start) + (0 smax (-127 + %start))) /u -127)
+; CHECK: Loop %for.body: max backedge-taken count is -128
 
 define void @slt_129_unknown_start(i8 %start) mustprogress {
 entry:
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===================================================================
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -11530,8 +11530,6 @@
                                                     unsigned BitWidth,
                                                     bool IsSigned) {
 
-  assert(!isKnownNonPositive(Stride) &&
-         "Stride is expected strictly positive!");
   // Calculate the maximum backedge count based on the range of values
   // permitted by Start, End, and Stride.
   const SCEV *MaxBECount;
@@ -11625,32 +11623,28 @@
     //
     // a) IV is either nuw or nsw depending upon signedness (indicated by the
     //    NoWrap flag).
-    // b) loop is single exit with no side effects.
+    // b) loop is single exit with no side effects and mustprogress.
     //
     //
     // Precondition a) implies that if the stride is negative, this is a single
     // trip loop. The backedge taken count formula reduces to zero in this case.
     //
-    // Precondition b) implies that the unknown stride cannot be zero otherwise
-    // we have UB.
+    // Precondition b) implies that if the stride is zero and rhs is invariant,
+    // this is a single trip loop.  The backedge taken count formula reduces
+    // to zero in this case.
     //
     // The positive stride case is the same as isKnownPositive(Stride) returning
     // true (original behavior of the function).
-    //
-    // We want to make sure that the stride is truly unknown as there are edge
-    // cases where ScalarEvolution propagates no wrap flags to the
-    // post-increment/decrement IV even though the increment/decrement operation
-    // itself is wrapping. The computed backedge taken count may be wrong in
-    // such cases. This is prevented by checking that the stride is not known to
-    // be either positive or non-positive. For example, no wrap flags are
-    // propagated to the post-increment IV of this loop with a trip count of 2 -
-    //
-    // unsigned char i;
-    // for(i=127; i<128; i+=129)
-    //   A[i] = i;
-    //
-    if (PredicatedIV || !NoWrap || isKnownNonPositive(Stride) ||
-        !loopIsFiniteByAssumption(L))
+    if (PredicatedIV || !NoWrap || !loopIsFiniteByAssumption(L))
+      return getCouldNotCompute();
+
+    // If we have a potentially zero step, and RHS isn't invariant in L, we
+    // don't know if it might eventually be greater than start.  The overflow
+    // fact does still imply an upper bound if we wanted to use that.  We
+    // could also try to prove start < end which would prove stride != 0 as
+    // a zero stride would be provably infinite and thus a contradition.
+    const bool StrideNonZero = isKnownNonZero(Stride);
+    if (!StrideNonZero && !isLoopInvariant(RHS, L))
       return getCouldNotCompute();
 
     // We allow a potentially zero stride, but we need to divide by stride
@@ -11679,7 +11673,7 @@
       auto *StartIfZero = getMinusSCEV(IV->getStart(), Stride);
       return isLoopEntryGuardedByCond(L, Cond, StartIfZero, RHS);
     };
-    if (!isKnownNonZero(Stride) && !wouldZeroStrideBeUB()) {
+    if (!StrideNonZero && !wouldZeroStrideBeUB()) {
       Stride = getUMaxExpr(Stride, getOne(Stride->getType()));
     }
   } else if (!Stride->isOne() && !NoWrap) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104140.359947.patch
Type: text/x-patch
Size: 4552 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210719/c6c46295/attachment.bin>