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

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

The changes look good to me @kmclaughlin! I'll look through the remaining non-aarch64 tests later today. I had a couple of minor comments so far ...



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9824
+  Instruction *I = getUnderlyingInstr();
+  if (State.VF.isVector() && IsUniform && isa<StoreInst>(I)) {
+    VPLane Lane = VPLane::getLastLaneForVF(State.VF);
----------------
This change looks sensible to me, since I think we mark unrolled scalar stores as uniform.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll:29
+; CHECK-NEXT:    br i1 [[TMP9]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK:       middle.block:
+; CHECK-NEXT:    [[CMP_N:%.*]] = icmp eq i64 [[N]], [[N_VEC]]
----------------
Perhaps it's worth deleting the CHECK lines from `middle.block` onwards as they don't add much value?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/sve-uniform-store.ll:5
+
+define void @uniform_store_i1(i1* noalias %dst, i64* noalias %start, i64 %N) {
+; CHECK-LABEL: @uniform_store_i1(
----------------
I wonder if it's perhaps worth moving these tests into sve-inv-store.ll, since they're testing the same thing?


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

https://reviews.llvm.org/D112725