[PATCH] D82227: SLP: honor requested max vector size merging PHIs

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D82227#2413359 <https://reviews.llvm.org/D82227#2413359>, @jonpa wrote:

> Would you like to give that a try? I think this patch could basically be reverted, and then some new hook would be needed to control MinVF/MaxVF in tryToVectorizeList(). Personally, I think it could be nice with something like TTI->getMaximumVF()  or even getMaximumSLPVF()... Probably also good if with a test case for your target if possible that is supposed to not produce any spilling...

I am experimenting with it now. I have reverted the patch and instead added this into tryToVectorizeList():

     unsigned MaxVF = std::max<unsigned>(PowerOf2Floor(VL.size()), MinVF);
  +  MaxVF = std::min(R.getMaxVecRegSize() / Sz, MaxVF);

I.e. clamp the list to the MaxVecRegSize. It should still allow reordering.

I'd say that solves the initial problem and probably a better solution overall. For instance I see less vectorization with AMDGPU where it should have not been vectorized in the first place.
However, the impact of that change will be less overall vectorization across all targets since tryToVectorizeList() is used not just for PHI.
That can be fixed by that new target callback if default value is MAX_UINT or just some large number. This default is somewhat subpar as I would expect a better default to be MaxVecRegSize, but I guess it can be decided per target.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82227/new/

https://reviews.llvm.org/D82227