[PATCH] D38494: [ScalarEvolution] Handling for ICmp occuring in the evolution chain.

[Jatin Bhateja via Phabricator via llvm-commits]

jbhateja added a comment.

@reviewerskindly let me know if any other comments



================
Comment at: lib/Analysis/ScalarEvolution.cpp:4092
+    const SCEV *Result = Rewriter.visit(S);
+    return Rewriter.isValid() ? Result : SE.getCouldNotCompute();
+  }
----------------
junryoungju wrote:
> It still can computable if after simplify loop forms.
> using SE.getCouldNotCompute() isn't a good idea.
> 
> see what happens on end of createAddRecFromPHI method.
Could not compute SCEV is returned when visitor could not compute ICmp. This is checked at client side for validating results returned by visitor. Hope I am able to put across my point.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6499
+
+const SCEV *ScalarEvolution::evaluateForICmp(ICmpInst *IC) {
+  BasicBlock *Latch = nullptr;
----------------
junryoungju wrote:
> It isn't really evoluting ICmp, just returning its true cond or false cond.
> I think we should change this method name more intuitively
Well in a way we are actually evaluating ICmp by matching it against the latch condition.  Shall rename to make it more intuitive to resolve your comment.


https://reviews.llvm.org/D38494