[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:2677
@@ -2748,6 +2676,3 @@
     } else if (Gep) {
       setDebugLocFromInst(Builder, Gep);
       // The last index does not have to be the induction. It can be
----------------
mssimpso wrote:
> delena wrote:
> > This change I made after buildbot failed at assertion. The main problem that I wasn't able to reproduce this failure. (llvm self build under sanitizer ..).
> > I fixed the failure by changing the first "if" from "if-induction" to "if-not-loop-invariant" and the buildbot passed all tests.
> > Do you think that this fix is correct?
> > May be you can tell me how to reproduce the failure?
> > Thanks.
> Hi Elena,
> 
> I'm looking at this for the first time, but it does look a little strange to me. Before your change, it looks like this case was specifically for pointer induction variables, but now it's not? If the GEP's pointer operand is not an induction variable, then for the GEP to be consecutive, the pointer operand should have to be loop invariant. We check this in isConsecutivePtr. Could this be a case of llvm::getGEPInductionOperand not agreeing with Legal->isInductionVariable?
> 
> Matt.
getelementptr %Ptr, %Ind1

Let me consider 2 cases (A) and (B):

(A)
Let assume that %Ptr is not induction and not loop-invariant. It, probably, means that Ptr is not changed from iteration to iteration, but defined inside the loop. LICM can't hoist it due to some reason. %Ind1 should be induction in this case.

(B)
Another case, Ptr is not induction, but it changes from iteration to iteration providing stride=1  for the memory access.
 Ptr = induction * X, for example, and memory versioning takes the case when X == 1. %Ind1 is loop invariant.

Case B seems more relevant than A.



Repository:
  rL LLVM

https://reviews.llvm.org/D20789