[PATCH] D23509: [LoopVectorize] Query TTI when deciding to splat IV

[Matthew Simpson via llvm-commits]

mssimpso added a comment.

Hi Sam,

I don't think TTI is the right way to fix this. The underlying issue in the test case is that we're vectorizing instructions marked uniform after vectorization. We should be scalarizing them instead.

This is what happens. We first collect the loop uniforms. The GEPs are marked uniform, as well as the arithmetic producing their indices. Because the IVs have no non-uniform users, they too are marked uniform, which all seems correct to me.

Then we vectorize the IVs. When vectorizing them, we know that we should only need scalar versions (Legal->isScalarAfterVectorization is true because the IVs are uniform). However, to keep WidenMap happy (see https://reviews.llvm.org/D23169) we actually vectorize them with the splat method as well. This is not ideal but also not the underlying issue. Because all users of the IVs should also be scalar, it's OK (but not good) that we vectorize them with the splat method since the vector versions would have no users and would be deleted later on in the clean-up phase.

Of course, the problem with this is that we actually still are vectorizing the uniform users of the IVs, creating uses of the vector IVs that we later can't remove. We shouldn't be doing this. I think the right fix is to have something like the following in each case in vectorizeBlockInLoop (except for PHIs and branches):

  if (Legal->isScalarAfterVectorization(&I)) {
    scalarizeInstruction(&I);
    continue;
  }

If we've already decided that an instruciton should remain scalar, we shouldn't vectorize it. Does this make sense? We will probably want to wait to do this until after https://reviews.llvm.org/D23169 has been committed to avoid large increases in compile-time. I tested the above approach (without https://reviews.llvm.org/D23169) on your test case and the ugly vector IVs were removed. Would you mind doing the same to see if your performance is restored?

Matt.


https://reviews.llvm.org/D23509