[PATCH] D22377: [SCEV] trip count calculation for loops with unknown stride

[Pankaj Chawla via llvm-commits]

pankajchawla added inline comments.

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8731-8783
@@ -8703,7 +8730,55 @@
 
-  // Avoid proven overflow cases: this will ensure that the backedge taken count
-  // will not generate any unsigned overflow. Relaxed no-overflow conditions
-  // exploit NoWrapFlags, allowing to optimize in presence of undefined
-  // behaviors like the case of C language.
-  if (!Stride->isOne() && doesIVOverflowOnLT(RHS, Stride, IsSigned, NoWrap))
+  // Avoid negative or zero stride values.
+  if (!PositiveStride) {
+    // We can compute the correct backedge taken count for loops with unknown
+    // strides if we can prove that the loop is not an infinite loop with side
+    // effects. Here's the loop structure we are trying to handle -
+    // 
+    // i = start
+    // do {
+    //   A[i] = i;
+    //   i += s;
+    // } while (i < end);
+    //
+    // The backedge taken count for such loops is evaluated as -
+    // (max(end, start + stride) - start - 1) /u stride
+    //
+    // The additional preconditions that we need to check to prove correctness
+    // of the above formula is as follows -
+    //
+    // a) IV is either nuw or nsw depending upon signedness (indicated by the
+    //    NoWrap flag).
+    // b) loop is single exit with no side effects.
+    //
+    //
+    // 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.
+    //
+    // 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) || 
+        !loopHasNoSideEffects(L))
+      return getCouldNotCompute();
+
+  } else if (!Stride->isOne() && doesIVOverflowOnLT(RHS, Stride, IsSigned, NoWrap))
+    // Avoid proven overflow cases: this will ensure that the backedge taken count
+    // will not generate any unsigned overflow. Relaxed no-overflow conditions
+    // exploit NoWrapFlags, allowing to optimize in presence of undefined
+    // behaviors like the case of C language.
     return getCouldNotCompute();
----------------
The comments for this function imply that it expects a positive stride-
`Verify if an linear IV with positive stride can overflow when...`

I think this makes sense because the condition in the loop latch is of the form (i < N). I added an assert to the function verifying that the incoming stride is indeed positive.

This check seems to be useless for unknown strides so I skip it. 

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8802
@@ +8801,3 @@
+  // for a loop with unknown stride.
+  APInt StrideForMaxBECount =
+      !PositiveStride ? APInt(BitWidth, 1, IsSigned)
----------------
I renamed `MinStride` to `StrideForMaxBECount`. I would have had to duplicate code to completely separate max backedge count computation for the two cases.


https://reviews.llvm.org/D22377