[PATCH] D28393: [SCEV] Make howFarToZero produce a smaller max backedge-taken count

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:7219
+      ConstantRange CR = getUnsignedRange(DistancePlusOne);
+      APInt NewMaxBECount = CR.getUnsignedMax() - One->getAPInt();
+      if (NewMaxBECount.ult(MaxBECount))
----------------
sanjoy wrote:
> sanjoy wrote:
> > Why are you checking `(Distance + 1) != 0` instead of `Distance != -1`?  I'd find the latter more straightforward.
> In case I gave the wrong impression, I don't specifically care about checking `(Distance + 1) != 0` vs. `Distance != -1`.  If you find the former more straightforward, then I'm okay too.
I went with `(Distance + 1) != 0` because it matches the condition which is likely to be written in the IR.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:7210
+    APInt MaxBECount = getUnsignedRange(Distance).getUnsignedMax();
+
+    // When a loop like "for (int i = 0; i != n; ++i) { /* body */ }" is rotated,
----------------
sanjoy wrote:
> I think we should really split this change up into two.
> 
>  - First the cleanup to handle count-up loops and count-down loops more uniformly (i.e. first reduce to `{D,+,-1}` and then compute `getUnsignedMax(D)`) than the current ad-hoc logic.  IIUC, this change is already an improvement for cases like `{X,+,1} != 0` if range(`X`) == `[-5, 1)` (say).
>  - Add the smartness around `unsigned_max(D) == unsigned_max(D + 1) - 1` on no overflow in a separate commit.
Okay, uploaded the first part of the split version.


Repository:
  rL LLVM

https://reviews.llvm.org/D28393