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

[Dorit Nuzman via Phabricator via llvm-commits]

dorit added a comment.

Hi Silviu,
I started to try out the approach you suggested, and I realized that our assumption doesn't hold... (see response to inlined comment).
Thanks,
Dorit



================
Comment at: lib/Analysis/ScalarEvolution.cpp:4482
+///   x1 = phi (x0, x_next)
+///   x2 = ExtTrunc (x1)
+///   x_next = add (x2, Step)
----------------
dorit wrote:
> sbaranga wrote:
> > If I understand correctly the SCEV expression returned by PSE for x2 should be the same as for x1?
> > 
> > In that case, wouldn't it be possible to search for values in the loop with the same SCEV as an induction PHI, go through the def-use chain until you've reached the PHI and mark all instructions in between as a (part of) a cast of that PHI? I think this wouldn't need any pattern matching and should be straight-forward to do.
> > 
> > Also I think ideally this should all be in an utility function (LoopUtils?). It seems a strange to have this in Scalar Evolution. It's very specific to the vectorizer.
> > If I understand correctly the SCEV expression returned by PSE for x2 should be the same as for x1?
> 
> Yes, that should be the case
> 
> > In that case, wouldn't it be possible to search for values in the loop with the same SCEV as an induction PHI
> 
> Is there a ScalarEvolution map that holds this information (all the Values that have the same SCEV)? There's the ExprValueMap which looks close, but I'm not sure... or do you suggest to really obtain "from scratch" all the values defined in the loop and call getSCEV for each? Would it make more sense to start from the Value that defines the phi from the backedge (the way we start now), walk the def-use chain from there? (calling getSCEV on each value we find on the way, and from the point where we encounter the same SCEV as the induction phi continue the def-use walk while marking instructions on the way as part of a cast)?
> 
> > Also I think ideally this should all be in an utility function (LoopUtils?). It seems a strange to have this in Scalar Evolution. It's very specific to the vectorizer.
> Ok, sure, will move it.
> 
> 
>  
> > If I understand correctly the SCEV expression returned by PSE for x2 should be the same as for x1?
> >
> Yes, that should be the case

Turns out that's not the case…. For example, for this def-use chain:
V0: %w_ix.011 = phi (0, %add)
V1: %sext = shl i64 %w_ix.011, 32
V2:  %idxprom = ashr exact i64 %sext, 32
V3:  %add = add i64 %idxprom, %step  

The scev expr of the Phi is {0,+,%step}<%for.body>
The scev expr of V2 is  (sext i32 (trunc i64 %w_ix.011 to i32) to i64)
 (this is what PSCEV.getSCEV() returns).... 


https://reviews.llvm.org/D38948