[PATCH] D100566: [SCEV] Add a ah-hoc pattern on isImpliedCondBalancedTypes

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:10468
+  // FoundRHS is AddRec's start value if and only if "AddRec == FoundRHS" is
+  // true. It means we can use "FoundRHS >= AddRec's start value".
+  if (FoundPred == ICmpInst::ICMP_EQ) {
----------------
nikic wrote:
> I find it really hard to understand this is doing based on the description. I believe the claim is that
> `A: Start s<= FoundRHS --> LHS s< RHS`
> implies
> `B: <Start,+,Step> == FoundRHS --> LHS s< RHS`.
> 
> A sufficient condition for that to hold would be that `<Start,+,Step> == FoundRHS` implies `Start s<= FoundRHS`. I assume that was your line of reasoning.
> 
> However, this is clearly not true without further legality checks. You need to at least require that the addrec is nsw, and that the step is non-negative.
I am sorry for poor description...

> A sufficient condition for that to hold would be that <Start,+,Step> == FoundRHS implies Start s<= FoundRHS. I assume that was your line of reasoning.

Yep, it is exactly correct.

> However, this is clearly not true without further legality checks. You need to at least require that the addrec is nsw, and that the step is non-negative.

You are right!!! Let me add legality checks with nsw and non-negative step.


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

https://reviews.llvm.org/D100566