[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.

In https://reviews.llvm.org/D38948#929665, @dorit wrote:

> IIUC, what IndVarSimply does is call the rewriter and then fix the users of the induction; This is similar in effect to what we do: We don't need to call the PSCEV rewriter again, we already have the nice AddRec for the induction phi (isInductionPhi() had already obtained it); The vectorization of the induction phi proceeds unchanged; The only thing we are adding is the def-use wiring so that in the vectorized loop, any users of the cast instructions will be users of the vectorized phi. We never vectorize the casts, and we never actively remove them -- they will end up dead code in the vectorized loop because they will not be used.


Yes, IndVarSimpify wouldn't fix this issue, but I was thinking more of using the techniques there that use the SCEV expressions to find these cases instead of doing the pattern matching (see the inline comment).

> 
> 
>> Comment at: lib/Analysis/ScalarEvolution.cpp:4470
>>  +/// (1) The value of the phi at iteration i is:
>>  +///      (Ext ix (Trunc iy ( Start + i*Step ) to ix) to iy)
>>  +/// (2) Under the runtime predicate, the above expression is equal to:
>> 
>>  ----------------
>> 
>> (1) might not hold in the future, so this might end up being a problem?
> 
> Do you mean this will not hold once additional forms of "casted-inductions" are supported by createAddRecFromPhiWithCasts()?

Yes, but even if it's not a correctness issue I think we should avoid pattern matching here if possible to avoid doing more work in the future.

Thanks,
Silviu



================
Comment at: lib/Analysis/ScalarEvolution.cpp:4482
+///   x1 = phi (x0, x_next)
+///   x2 = ExtTrunc (x1)
+///   x_next = add (x2, Step)
----------------
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.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4491
+///   ExtTrunc2: 
+///     Cast:  %t = shl %x1, m
+///     Cast:  %x2 = ashr %t, m
----------------
Technically this would be part of a cast operation, and not a cast itself? (shl changes the value).


https://reviews.llvm.org/D38948