[PATCH] D20789: Consecutive memory access in Loop Vectorizer - fixed and simplified

[Adam Nemet via llvm-commits]

anemet added inline comments.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2158-2176
@@ -2157,13 +2157,21 @@
 
 int LoopVectorizationLegality::isConsecutivePtr(Value *Ptr) {
+
+  if (getSymbolicStrides()) {
+    int Stride = getPtrStride(PSE, Ptr, TheLoop, *(getSymbolicStrides()), true,
+                              false);
+    if (Stride == 1 || Stride == -1)
+      return Stride;
+    return 0;
+  }
   assert(Ptr->getType()->isPointerTy() && "Unexpected non-ptr");
   auto *SE = PSE.getSE();
   // Make sure that the pointer does not point to structs.
   if (Ptr->getType()->getPointerElementType()->isAggregateType())
     return 0;
 
   // If this value is a pointer induction variable, we know it is consecutive.
   PHINode *Phi = dyn_cast_or_null<PHINode>(Ptr);
   if (Phi && Inductions.count(Phi)) {
     InductionDescriptor II = Inductions[Phi];
     return II.getConsecutiveDirection();
----------------
Also you didn't resolve the merge conflict properly.  isConsecutivePtr was just forwarding to getPtrStride in the earlier version as it should but now the entire original body has snuck back in.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2160-2162
@@ +2159,5 @@
+
+  if (getSymbolicStrides()) {
+    int Stride = getPtrStride(PSE, Ptr, TheLoop, *(getSymbolicStrides()), true,
+                              false);
+    if (Stride == 1 || Stride == -1)
----------------
No, this should be something like:

const ValueToValueMap &Strides = getSymbolicStrides() ? *getSymbolicStrides() : ValueToValueMap();

int Stride = getPtrStride(PSE, Ptr, TheLoop, Strides, true, false);

================
Comment at: ../test/Transforms/LoopVectorize/consec_no_gep.ll:27-35
@@ +26,11 @@
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %i.05 = phi i32 [ %inc, %for.body ], [ 0, %for.body.preheader ]
+  %from.addr.04 = phi float* [ %incdec.ptr, %for.body ], [ %from, %for.body.preheader ]
+  %to.addr.03 = phi float* [ %incdec.ptr1, %for.body ], [ %to, %for.body.preheader ]
+  %incdec.ptr = getelementptr inbounds float, float* %from.addr.04, i64 1
+  %0 = bitcast float* %from.addr.04 to i32*
+  %1 = load i32, i32* %0, align 4
+  %incdec.ptr1 = getelementptr inbounds float, float* %to.addr.03, i64 1
+  %2 = bitcast float* %to.addr.03 to i32*
+  store i32 %1, i32* %2, align 4
+  %inc = add nsw i32 %i.05, 1
----------------
As I said earlier, the float cast is non-essiential here.  Please simplify this just to use integer pointers.


Repository:
  rL LLVM

http://reviews.llvm.org/D20789