[PATCH] D37265: [SCEV] Ensure ScalarEvolution::createAddRecFromPHIWithCastsImpl properly handles out of range truncations of the start and accum values

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4516
+// OPTION to be removed before landing.
+// To the reviewers: Which version do we want to use?
+//  I suspect that it's highly unlikely that anything other than
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > mkazantsev wrote:
> > > For me, this option looks more reasonable.
> > >     return Expr != ExtendedExpr &&
> > >            isKnownPredicate(ICmpInst::ICMP_NE, Expr, ExtendedExpr);
> > > 
> > By the way. I grew hesitant about the `Expr != ExtendedExpr` condition. If two SCEVs don't match pointer-wise, it doesn't mean that they are not equal. For example, constant `2` and `2` which is load from a field are equal, but you will have a `SCEVConstant` in one case and a `SCEVUnknown` in another case. I think that the correct option would be
> >   return isKnownPredicate(ICmpInst::ICMP_NE, Expr, ExtendedExpr);
> It's not the case in your situation, though. So maybe it's OK, but using just `isKnownPredicate` looks more safe.
Scratch this, I've re-read the code and realised that what I wrote is bizzare. 
  return Expr != ExtendedExpr &&
         isKnownPredicate(ICmpInst::ICMP_NE, Expr, ExtendedExpr);
is ok.


https://reviews.llvm.org/D37265