[PATCH] D143409: [SCEV][IndVarSimplify] Support known dominating slt/ult elimination

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:11115
+    // - If ArLHS is negative, 'ArLHS <u RHS' will be false on the first
+    // iteration
+    if (isKnownPositive(ArLHS->getStepRecurrence(*this)) &&
----------------
mkazantsev wrote:
> Sure this is wrong.
> ```
> start = SINT_MAX - 1
> step = 2
> RHS = SINT_MAX
> ```
> 
> On first iteration, both checks pass.
> ```
> SINT_MAX - 1 <s SINT_MAX // true
> SINT_MAX - 1 <u SINT_MAX // true
> ```
> On the 2nd iteration unsigned check will fail.
> SINT_MAX + 1 <s SINT_MAX // true
> SINT_MAX + 1 <u SINT_MAX  // false
> 
> Note that what you did is just duplicated the checks above, but dropped the `nuw` check that was there for reason.
Sorry, `nsw` check.


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

https://reviews.llvm.org/D143409