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

[Elena Demikhovsky via llvm-commits]

delena added inline comments.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2136-2137
@@ -2146,2 +2135,4 @@
+  auto *SE = PSE.getSE();
+  if (Gep->getNumOperands() != 2)
     return 0;
 
----------------
mzolotukhin wrote:
> Can we relax this constraint? Looks like we didn't have such limitation before.
You are right, but this index should be the last. Otherwise step 1 will not mean consecutive GEP.
I'll fix.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2140
@@ -2169,1 +2139,3 @@
+  // Induction operand is the index of the GEP, not a pointer.
   unsigned InductionOperand = getGEPInductionOperand(Gep);
+  if (InductionOperand != 1 ||
----------------
mkuper wrote:
> Are you planning to call this with more complex GEPs?
> If not, I'm not sure I understand why we need to call getGEPInductionOperand(Gep) here. Isn't it guaranteed to return 1?
It was original code. I assume that I have to check two things - (1) The last index in "induction".  (2) All other are loop-invariant.


================
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;
   }
----------------
Ayal wrote:
> 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().
> we could use it to return the desired stride instead of zero
I don't want to complicate the code and make it more general since I don't see other usage for it.
I can put this function back to isConsecutivePtr() if you prefer this solution.

================
Comment at: ../test/Transforms/LoopVectorize/consec_no_gep.ll:6
@@ +5,3 @@
+
+;; Check consecutive memory access without preceding GEP instruction
+
----------------
mzolotukhin wrote:
> Could you please elaborate on what do you mean by "without preceding GEP instruction"? The test below has two GEP-instructions before load and store. Is the problem in the bitcast between GEP and load/store?
The bitcast is between "load" and "Phi". GEP does not feed load in this case.


Repository:
  rL LLVM

http://reviews.llvm.org/D20789