[PATCH] D22035: [LoopVectorizer] Fixed a bug in gather/scatter intrinsics.

[Ayal Zaks via llvm-commits]

Ayal added a comment.

This looks good to me, with minor nits.


================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2696-2697
@@ -2695,4 +2695,4 @@
       SmallVector<VectorParts, 4> OpsV;
-      // Vectorizing GEP, across UF parts, we want to keep each loop-invariant
-      // base or index of GEP scalar
+      // Vectorizing GEP, across UF parts, we want to keep index scalar
+      // if it is not defined inside the loop.
       for (Value *Op : Gep->operands()) {
----------------
Better emphasize that:
we want to get a vector value for base and each index that's defined inside the loop, even if it is loop-invariant but wasn't hoisted out.  Otherwise we want to keep them scalar.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2703-2705
@@ +2702,5 @@
+        else {
+          VectorParts Scalars;
+          Scalars.append(UF, Op);
+          OpsV.push_back(Scalars);
+        }
----------------
Can simply keep the original OpsV.push_back(VectorParts(UF, Op)), right?

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2714
@@ -2712,1 +2713,3 @@
+        Value *NewGep =  Builder.CreateGEP(GEPBasePtr, Ops, "VectorGep");
+        cast<GetElementPtrInst>(NewGep)->setIsInBounds(Gep->isInBounds());
         assert(NewGep->getType()->isVectorTy() && "Expected vector GEP");
----------------
Just noting that this is an independent, obvious, fix.

================
Comment at: ../test/Transforms/LoopVectorize/X86/scatter_crash.ll:12
@@ +11,3 @@
+; Function Attrs: norecurse nounwind ssp uwtable
+define void @_Z3fn1v() #0 {
+; CHECK-LABEL: @_Z3fn1v(
----------------
Indicate more clearly what this test is designed to check for?


Repository:
  rL LLVM

http://reviews.llvm.org/D22035