[PATCH] D38948: [LV] Support efficient vectorization of an induction with redundant casts

[Dorit Nuzman via Phabricator via llvm-commits]

dorit marked 3 inline comments as done.
dorit added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4674
+  //  NSSW or NUSW)
+  const SCEV *AccumExtended = GetExtendedExpr(Accum, /*Signed=*/true);
   if (PredIsKnownFalse(Accum, AccumExtended)) {
----------------
sbaranga wrote:
> Nice catch, I think you can do a separate commit for this fix.
Done - this is https://reviews.llvm.org/D40641.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4740
 }
 
+bool PredicatedScalarEvolution::areAddRecsEqualWithPreds (
----------------
sbaranga wrote:
> Could you add a FIXME here?
> 
> This shouldn't be required, and PSE should return the same expression for both.
Yes, I'll do that; 
"FIXME: This is currently required because the rewriter currently does not rewrite this: 
 {0, +, (sext ix (trunc iy to ix) to iy)} 
 into {0, +, %step},
although there exists an Equal predicate "%step ==  (sext ix (trunc iy to ix) to iy)".



================
Comment at: lib/Analysis/ScalarEvolution.cpp:4748
+      if (Expr1 != Expr2 &&
+          !SE.isKnownPredicate(ICmpInst::ICMP_EQ, Expr1, Expr2) &&
+    		  !Preds.implies(SE.getEqualPredicate(Expr1, Expr2)) &&
----------------
sbaranga wrote:
> Any idea what the complexity of isKnownPredicate() is?
I guess it can be potentially more complex than looking up Preds.implies... I can move it to be last check (or drop it...?). 


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:889
+  ScalarEvolution *SE = PSE.getSE();
+  if (!SE->havePredicatedSCEVRewriteForPHI(PhiScev, AR))
+    return false;
----------------
sbaranga wrote:
> I guess this is just an optimization for speed?
> 
> Maybe it's enough to check that SE returns a SCEVUnknown and PSE returns an AddRec?
Yes, to make sure we only look at relevant phis...

The check you suggest is exactly what the caller to this utility checks before calling this routine... 


https://reviews.llvm.org/D38948