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

[Eli Friedman via Phabricator via llvm-commits]

efriedma 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);
----------------
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?


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106197