[PATCH] D101076: [SVE][LoopVectorize] Add support for scalable vectorization of first-order recurrences

[David Sherwood via Phabricator via llvm-commits]

david-arm accepted this revision.
david-arm added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks for the changes - it looks like you've addressed everyone's comments.



================
Comment at: llvm/lib/IR/IRBuilder.cpp:1036
+
+  if (auto VTy = dyn_cast<ScalableVectorType>(V1->getType())) {
+    Module *M = BB->getParent()->getParent();
----------------
nit: Maybe fix clang-tidy warning before merging?


================
Comment at: llvm/lib/IR/IRBuilder.cpp:1053
+  SmallVector<int, 8> Mask;
+  for (unsigned i = 0; i < NumElts; ++i)
+    Mask.push_back(Idx + i);
----------------
nit: Maybe fix clang-tidy warning before merging?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4228
+        Incoming, Idx, "vector.recur.extract.for.phi");
+    // When loop is unrolled without vectorizing, initialize
+    // ExtractForPhiUsedOutsideLoop with the value just prior to unrolled value
----------------
nit: It looks like these comments have been included in the vector case, but I think they're intended for the scalar **else if** block below. Perhaps you can just move them into that block instead?


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

https://reviews.llvm.org/D101076