[PATCH] D87832: [IndVars] Remove monotonic checks with unknown exit count

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:9248
+    ICmpInst::Predicate Pred, const SCEV *LHS, const SCEV *RHS, const Loop *L,
+    const Instruction *Context, const SCEV *MaxIter,
+    ICmpInst::Predicate &InvariantPred, const SCEV *&InvariantLHS,
----------------
mkazantsev wrote:
> apilipenko wrote:
> > Do we actually need to pass MaxIter? We already have the loop, we can check the max iterations for the loop inside of this function. 
> > 
> > If we make this interface change, isLoopInvariantPredicateAtDuringFirstIterations interface becomes very similar to isLoopInvariantPredicate. At that point we can add context to isLoopInvariantPredicate and implement the proposed logic in isLoopInvariantPredicate.
> We do not necessarily want to evaluate the last iteration. In some cases (and this is in follow-up patches), if we prove that there are two exits with same iteration count, the 2nd one will not execute on the last iteration. So we might want to check the condition for pre-last iteration for it.
Besides, before this code was written, max iter computation was a costly operation (I've added caching for it just today). But per argument above, I don't think we want to compute it inside.

I agree that `isLoopInvariantPredicate` might be remade into calling this function with last iteration. Let's not to API changes in the patches that are not about API changes. It can go separately after this work has concluded.


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

https://reviews.llvm.org/D87832