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

[Ayal Zaks via llvm-commits]

Ayal added inline comments.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2130-2133
@@ -2129,15 +2129,6 @@
 
-int LoopVectorizationLegality::isConsecutivePtr(Value *Ptr) {
-  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();
-  }
+static int getStrideFromGEPIndex(PredicatedScalarEvolution &PSE,
+                                 GetElementPtrInst *Gep,
+                                 const Loop *Lp,
+                                 const ValueToValueMap &StridesMap) {
 
----------------
It's admittedly not your original code, but while you're refactoring this "gep with all indices being invariant except last that's inductive with a step that's not originally a compile time constant but gets replaced by one due to specialization, skipping over z/sext/bitcast" stride 1 special case, should it belong to LAA's getPtrStride() rather than LVL's isConsecutivePtr()?

In any case, please add documentation explaining what this is about.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2167-2170
@@ -2205,6 +2166,6 @@
     // and all other indices are loop invariant.
     if (Step->isOne())
       return 1;
     if (Step->isAllOnesValue())
       return -1;
   }
----------------
Again, admittedly not your original code, just commenting that if Step is any constant, not only +-1, we could use it to return the desired stride instead of zero. Or perhaps we'd only be specializing for the final stride(?). Otherwise you may call this isConsecutiveGEPIndex() rather than getStrideFromGEPIndex().


Repository:
  rL LLVM

http://reviews.llvm.org/D20789