[PATCH] D32166: Improve LoopVectorizers estimation of scalar loops by predicting LSR behaviour

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

In https://reviews.llvm.org/D32166#743732, @rengolin wrote:

> Hi Jonas,
>
> As much as this helps, I think duplicating the LSR logic here, even if a very limited subset of it, is problematic.
>
> If assumptions change with time, you may start having increasingly different results and not detect them until the more pathological cases start to explode.
>
> In cases like this, in the past, we discussed splitting the analysis pass from the execution pass on the targeted transformation. IIRC, this is how the loop analysis passes were born.
>
> Maybe, if you move the LSR analysis code to the generic loop pass side, and both LSR and LV use the same analysis, you can work around the issue without duplicating anything.
>
> Makes sense?
>
> cheers,
> --renato


Hi Renato,

thanks for your opinion!

Ideally the LSR should handle vectorized addresses as well, and if it could, there would not be this difference against the scalar loops. It however can't, and that's why this patch improves on vectorizer decions by letting the scalar loop have a cheaper cost in these cases.

Is LSR going to do this anytime soon? Or is it an option to run LSR (also?) before the Loop vectorizer? I have seen many cases where the scalar loop is much smaller just because the vectorizer generates a vector add, vector shift and what not of the address, while LSR removes those instructions entirely in the scalar loop. In this sense, isn't the loop vectorizer run too early?

I am not sure if it's worth the effort to factor out the LSR analysis just to know how much better it is on scalar loops... It would be better to fix the vectorized loops instead. So I guess, this patch should actually just hopefully be temporary if used...


https://reviews.llvm.org/D32166