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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

I think the title needs updating after the latest update (remove `SVE`).



================
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:
> > 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.
If D113034  would be landed first, shouldn't `IsUniform` be set correctly? Ideally the uniform information should be explicit in the recipe and Legal should not be accessed during codegen.


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

https://reviews.llvm.org/D112725