[PATCH] D12073: Make ScalarEvolution::isKnownPredicate a little smarter

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

I'd solve this differently -- if I understand the problem correctly, you should really only need to teach SCEV that `{0,+,1}<nsw>` is `slt` `{1,+,1}<nsw>` (or something similar to that) in `isImpliedCondOperandsHelper`.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6873
@@ -6872,3 +6872,3 @@
 bool
 ScalarEvolution::isKnownPredicateWithRanges(ICmpInst::Predicate Pred,
                                             const SCEV *LHS, const SCEV *RHS) {
----------------
This should probably live in `ScalarEvolution::isKnownPredicate` or `ScalarEvolution::isImpliedCondOperandsHelper`, not here.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6892
@@ -6891,1 +6891,3 @@
       return false;
+
+    const SCEV *Diff = getMinusSCEV(LHS, RHS);
----------------
I'm not sure this is sound w.r.t. overflow.  For instance, if `LHS` = `i8 127` and `RHS` = `i8 -1` then `!(LHS < RHS)` but `LHS - RHS = i8 128 < 0`

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6909
@@ +6908,3 @@
+
+    const SCEV *Diff = getMinusSCEV(LHS, RHS);
+    if (isKnownNonPositive(Diff))
----------------
Same comment as above on overflow.


Repository:
  rL LLVM

http://reviews.llvm.org/D12073