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

[silviu.baranga@arm.com via Phabricator via llvm-commits]

sbaranga added a comment.

Hi Dorit,

Thanks for doing the changes! I just have a few comments/questions (see inline).

Silviu



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


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4740
 }
 
+bool PredicatedScalarEvolution::areAddRecsEqualWithPreds (
----------------
Could you add a FIXME here?

This shouldn't be required, and PSE should return the same expression for both.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4748
+      if (Expr1 != Expr2 &&
+          !SE.isKnownPredicate(ICmpInst::ICMP_EQ, Expr1, Expr2) &&
+    		  !Preds.implies(SE.getEqualPredicate(Expr1, Expr2)) &&
----------------
Any idea what the complexity of isKnownPredicate() is?


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:889
+  ScalarEvolution *SE = PSE.getSE();
+  if (!SE->havePredicatedSCEVRewriteForPHI(PhiScev, AR))
+    return false;
----------------
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?


https://reviews.llvm.org/D38948