[PATCH] D112725: [SVE][LoopVectorize] Extract the last lane from a uniform store

[Kerry McLaughlin via Phabricator via llvm-commits]

kmclaughlin added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9818
+  Instruction *I = getUnderlyingInstr();
+  if (!IsUniform && State.VF.isScalable() && isa<StoreInst>(I) &&
+      State.ILV->getLegal()->isUniformMemOp(*I)) {
----------------
sdesmalen wrote:
> sdesmalen wrote:
> > While it is a concrete problem for scalable vectors, I don't think this is necessarily specific to scalable vectors and so we may want to do the same thing for fixed-width vectors. I'd expect other passes to remove the redundant scalar stores that are currently created, but it would be nice if those would not be generated in the first place.
> I would expect `isScalarAfterVectorization` to be set to `true` when the memory address is uniform and that to be used instead of '`!IsUniform && isUniformMemOp(*I)`. Can you check whether this can be used instead?
Hi @sdesmalen, I've removed `State.VF.isScalable()` and added a new test for fixed-width to LoopVectorize/uniform-store.ll. There were also a few existing tests with uniform stores affected by this change which have been updated.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9818
+  Instruction *I = getUnderlyingInstr();
+  if (!IsUniform && State.VF.isScalable() && isa<StoreInst>(I) &&
+      State.ILV->getLegal()->isUniformMemOp(*I)) {
----------------
kmclaughlin wrote:
> sdesmalen wrote:
> > sdesmalen wrote:
> > > While it is a concrete problem for scalable vectors, I don't think this is necessarily specific to scalable vectors and so we may want to do the same thing for fixed-width vectors. I'd expect other passes to remove the redundant scalar stores that are currently created, but it would be nice if those would not be generated in the first place.
> > I would expect `isScalarAfterVectorization` to be set to `true` when the memory address is uniform and that to be used instead of '`!IsUniform && isUniformMemOp(*I)`. Can you check whether this can be used instead?
> Hi @sdesmalen, I've removed `State.VF.isScalable()` and added a new test for fixed-width to LoopVectorize/uniform-store.ll. There were also a few existing tests with uniform stores affected by this change which have been updated.
I tried removing `!IsUniform && isUniformMemOp(*I)` and replacing it with `isScalarAfterVectorization`, though this returns false for the instruction here and we continue on to hit the "Can't scalarize a scalable vector" assert below.


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

https://reviews.llvm.org/D112725