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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7470-7471
         } else {
-          assert((isa<LoadInst>(&I) || !VF.isScalable()) &&
+          assert((isa<LoadInst>(&I) || !VF.isScalable() ||
+                  Legal->isUniformMemOp(I)) &&
                  "Cannot yet scalarize uniform stores");
----------------
This condition is always true, because it is enclosed by `if (Legal->isUniformMemOp(I))`


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9817
 
+  Instruction *I = getUnderlyingInstr();
+  if (!IsUniform && State.VF.isScalable() && isa<StoreInst>(I) &&
----------------
This code needs a comment with rationale for extracting the last lane.


================
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)) {
----------------
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.


================
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:
> 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112725