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

[Philip Reames via Phabricator via llvm-commits]

reames updated this revision to Diff 359395.
reames added a comment.
Herald added a subscriber: xgupta.

Rebase over tests which actually exercise code and show difference.

The zero stride issue was fixed separately.  This still depends on Eli's patch for overflow correctness.  Once that's fixed, I think this will be safe.

It's worth noting that given a condition which dominates the latch, an IV which has either nsw or nuw, and a negative step, the exit backedge taken must be either 0 or 1.  For it to be more than that, we must perform at least two adds of negative value which must by definition wrap.  As a result, we could do much better in terms of formulas specific for negative strides, but I'm not really interested in bothering.  The main value of this patch is in making it easier to exercise the general path via tests.


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
@@ -11541,8 +11541,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;
@@ -11626,32 +11624,18 @@
     //
     // 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, 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();
 
     // We allow a potentially zero stride, but we need to divide by stride


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