[PATCH] D39346: [LV] [ScalarEvolution] Fix PR34965 - Cache pointer stride information before LV code gen

[Hideki Saito via Phabricator via llvm-commits]

hsaito added a comment.

In https://reviews.llvm.org/D39346#917815, @rengolin wrote:

> In https://reviews.llvm.org/D39346#915527, @dcaballe wrote:
>
> > I implemented a new fix where the return value of Legal->isConsecutivePtr() is collected just before the original loop is modified. This approach seems simpler and less invasive than the one described in my previous comment.
>
>
> I may not have understood the problem correctly, but I think we shouldn't moved cached analysis into new representations.
>
> To me it seems that SCEV is not able to see something that has a dependency, which VPlan has just exposed. In this case, fixing/extending SCEV is the right thing to do.
>
> I'm adding the VPlan folks to get a wider audience.
>
> cheers,
> --renato


There are two things. If we are expecting to see the same result coming back, we shouldn't re-analyze. If we are expecting to see better result, there is a good reason to re-analyze.
Memory reference kind information (unit-stride, strided, or gather) is utilized in multiple places in vectorizer, and failing to see the same/better result can lead to vectorizer's stability
problem. So, having such data handy (i.e., cache) is a good defensive programming. As we improve vectorizer's capability, we expect memory reference kind information becomes more
and more complex (e.g., unit-stride up to certain VF, piece-wise unit-stride beyond that). Re-analyzing that will become more and more undesirable. So, please consider the tentative fix
as a baby step towards that direction.

Fixing/extending SCEV is also a good thing to do ---- and if vectorizer is re-analyzing, we should add an assertion such that the resulting information hasn't degraded from the cached info.

Another thing that caught my attention was Legal->isUniform() invalidating SCEV for %11. I don't think it should. Having said that, Simon Moll (Saarland) and we (Intel team) are looking into bringing Divergence Analysis from RV project. As such, we currently do not have a good incentive to dig deeper into Legal->isUniform() to resolve this particular issue.

Thanks,
Hideki


https://reviews.llvm.org/D39346