[PATCH] D13042: [SCEV] Try to prove predicates by splitting them.

[hfinkel@anl.gov via llvm-commits]

hfinkel added inline comments.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6981
@@ +6980,3 @@
+  if (Pred == ICmpInst::ICMP_ULT && isKnownNonNegative(RHS) &&
+      isKnownPredicate(CmpInst::ICMP_SGE, LHS,
+                       getConstant(LHS->getType(), 0)) &&
----------------
sanjoy wrote:
> hfinkel wrote:
> > Why do you use isKnownNonNegative(RHS) but not isKnownNonNegative(LHS)?
> > 
> `isKnownNonNegative` is less powerful than `isKnownPredicate`.  `isKnownPredicate` does a control dependence analysis while `isKnownNonNegative` just looks at `getSignedRange` which does not take most control dependence into account (it does take //some// control dependence into account indirectly, by leveraging the no-wrap flags on SCEV instructions).
> 
> The cases we're looking to optimize are of the form `%iv ult %length`.  Here `%length` is a load instruction with `!range` metadata that lets us directly prove that `%length` is positive without control dependence, and `%iv` is an induction variable which takes some control dependence analysis to prove as being always positive.
> 
> I cannot try this out before tomorrow, but I think using `isKnownPredicate` to prove `RHS` is positive as well should work (except we'll be burning more compile time).  Another possibility is to keep the code as is, and add what I said here as a comment.
I'd only recommend doing the more expensive thing if you can construct a test case where it helps, and that test case is in canonical form (i.e. IndVarSimplify would not change it to make the simpler version work). Otherwise, just adding what you have here as a comment should be fine.



http://reviews.llvm.org/D13042