[PATCH] D106197: [ScalarEvolution] Refine computeMaxBECountForLT to be accurate in more cases.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:11553
+  APInt One(BitWidth, 1);
+  StrideForMaxBECount = IsSigned ? APIntOps::smax(One, StrideForMaxBECount)
+                                 : APIntOps::umax(One, StrideForMaxBECount);
----------------
efriedma wrote:
> reames wrote:
> > This is harmless, but I believe we've already established this by the time this method is invoked.   
> At the moment, yes, but we're probably going to drop that check?
I'd meant in the bit of code which forces stride to umin(1,stride) if we can't prove it non-zero in the caller.  


================
Comment at: llvm/test/Analysis/ScalarEvolution/max-trip-count.ll:459
+
+define void @max_overflow(i8 %n) mustprogress {
+; CHECK-LABEL: Determining loop execution counts for: @max_overflow
----------------
efriedma wrote:
> reames wrote:
> > Can you add a test case for the i1 case?  I had to write it out to convince myself of your comment and I think having the test in the change would help clarify for later readers.
> I can add the test, but it will just return CouldNotCompute without further changes; `isKnownNonPositive(Stride)` is true if Stride is an i1.
Please add it anyways.  :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106197