[PATCH] D38825: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev requested changes to this revision.
mkazantsev added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9693
+        IsSigned ? getSignedRangeMin(Stride) : getUnsignedRangeMin(Stride);
+  else
+    // Using a stride of 1 is safe when computing max backedge taken count for
----------------
If the stride is known negative and `IsSigned` is `true`, can we return `computeMaxBECount(End, -Stride, Start, BitWidth, IsSigned)` instead of conservative estimate?


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9694
+  else
+    // Using a stride of 1 is safe when computing max backedge taken count for
+    // a loop with unknown stride.
----------------
Do we have a guarantee that stride is not zero? I think we should bail early unless we proved it or assert on that.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9710
+  MaxBECount = computeBECount(getConstant(MaxEnd - MinStart),
+                              getConstant(StrideForMaxBECount), false);
+
----------------
Please add name for the last parameter in `/* */`


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9808
+  if (!isLoopInvariant(RHS, L)) {
+    const SCEV *MaxBECount = computeMaxBECount(
+        Start, Stride, RHS, getTypeSizeInBits(LHS->getType()), IsSigned);
----------------
Do we have a guarantee that the IV increment has no wrap flag set? Otherwise I don't believe it works for stride other than 1. For example:

  L = load volatile ...
  for (int i = 0; i < L; i += 2) {
    ...
    L = load volatile ...
  }

If `L` is always `SINT_MAX`, this loop is infinite (if IV increment doesn't have `nsw` flag set).

For stride = 1 I think it should work fine even without `nsw`: we always stop when `i = SINT_MAX` at most, but for other step values we can leap across it.


================
Comment at: test/Analysis/ScalarEvolution/max-trip-count.ll:345
+; CHECK: Loop %loop: Unpredictable backedge-taken count.
+; CHECK: Loop %loop: max backedge-taken count is 1073741823
+entry:
----------------
Please add a test that if `iv.next` does not have `nsw` flag for this example then the max backage-taken count is unpredictable (this loop is infinite if `start = 0`, `n` is always `SINT_MAX`) because we leap across SINT_MAX.


https://reviews.llvm.org/D38825