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

Kerry McLaughlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 2 11:25:42 PDT 2021


kmclaughlin marked an inline comment as not done.
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)) {
----------------
kmclaughlin wrote:
> 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.
@sdesmalen I think you're right that `isScalarAfterVectorization` should be true for the store instruction and I've created D113034 (which changes `collectLoopUniforms` to also consider uniform stores) to address this.
I think we still need to check `isUniformMemOp` here though, since Scalars collects more than just uniform instructions and we only want to generate the last lane for uniform stores.


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

https://reviews.llvm.org/D112725



More information about the llvm-commits mailing list