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

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

sbaranga added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:889
+  ScalarEvolution *SE = PSE.getSE();
+  if (!SE->havePredicatedSCEVRewriteForPHI(PhiScev, AR))
+    return false;
----------------
dorit wrote:
> 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... 
In that case would we ever bail out here? If not, and this doesn't affect correctness, maybe we shouldn't do this check?

The problem I have with havePredicatedSCEVRewriteForPHI is that it is looking into the SCEV internal structures while SCEV is doing the analysis lazily. So the answer that it's getting is that either we don't have a better value under some predicates or we haven't performed the analysis at all. So maybe there is an assumption here that createAddRecFromPHIWithCasts was called at some point.

>From an interface perspective, I think it's better to call createAddRecForPHIWithCasts if we really require doing this check.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:943
+    auto *AddRec = dyn_cast<SCEVAddRecExpr>(PSE.getSCEV(Val));
+    if (AddRec && PSE.areAddRecsEqualWithPreds(AddRec,AR)) {
+      InCastSequence = true;
----------------
nit: formatting, there should be a space before after AR


https://reviews.llvm.org/D38948