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

> Maybe I'm missing something, but why not rewrite the induction variable using the the SCEV that we get from PSE (something like what IndVarsSimpify does)? If we would take this approach then the casts would end up being dead code and removed.

We can't do that on the original loop, which remains scalar and untouched, and will not be guarded by the scev predicates.
This means that the cast instructions will be there when we start vectorizing, and we need a way to tell that they will cost us nothing, and that we don't need to vectorize them but rather "look through them" as if they weren't there.

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.

> 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()?
In such a case this function will not find the IR sequence that it expects, and we will have a missed optimization (namely, the vectorizer will vectorize the cast instructions instead of ignoring them). But we won't have a correctness problem.

I think I should probably add a comment to the documentation of createAddRecFromPhiWithCasts() to remind us that if/when it is extended to support more forms of "casted-induction" SECV-Exprs, then it is recommended to also extend getCastsForInductionPHI() to look for the additional IR patterns that can result in these SCEV-Exprs. Would that address your concern?

Thanks,
Dorit


https://reviews.llvm.org/D38948