[PATCH] D24557: [Loop Vectorizer] Simplified GEP cloning. NFC.

[Adam Nemet via llvm-commits]

anemet added inline comments.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2790-2791
@@ +2789,4 @@
+      Gep2->setName("gep.indvar");
+      for (unsigned i = 0; i < NumOperands; ++i)
+        Gep2->setOperand(i, getScalarValue(Gep->getOperand(i), 0, 0));
+      setDebugLocFromInst(Builder, Gep);
----------------
mssimpso wrote:
> anemet wrote:
> > @delena, to be a bit more specific, the comment I would except to see here is something like:
> > 
> > 
> > 
> > > Let's create the uniform value of this GEP (i.e. lane 0 value).  The operands are either loop-invariant or consecutive (GEP pointer or induction-variable index). 
> > > 
> > 
> > 
> > 
> > @mssimpso, please check that I am not oversimplifying/missing some subtleties here again.
> Sounds mostly good to me! For "GEP pointer or induction variable index", I may be taking things a bit too far than is useful, but I think the consecutive pointer operand (if non-pointer IV) doesn't technically have to be a GEP, since we still look through bitcasts (with getGEPInstruction) in isConsecutivePtr. What about something like:
> 
> "One operand is either a non-pointer induction variable or consecutive pointer (pointer induction variables are also consecutive), and the other operands are loop-invariant."
> 
> I think it would also be useful to comment on the fact that this GEP is created for the first unroll iteration and that the GEPs for the rest of the unroll iterations are computed below as an offset from this GEP.
> Sounds mostly good to me! For "GEP pointer or induction variable index", I may be taking things a bit too far than is useful, but I think the consecutive pointer operand (if non-pointer IV) doesn't technically have to be a GEP, since we still look through bitcasts (with getGEPInstruction) in isConsecutivePtr.

Are you talking about the pointer operand of this 'Gep' (as the variable in the code)?  I didn't mean to suggest that that is produced by a GEP, I was just referring to the pointer operand of the variable 'Gep'.



> "One operand is either a non-pointer induction variable or consecutive pointer (pointer induction variables are also consecutive), and the other operands are loop-invariant."
> 
> I think it would also be useful to comment on the fact that this GEP is created for the first unroll iteration and that the GEPs for the rest of the unroll iterations are computed below as an offset from this GEP.

Agreed and I think that is an implication that this is a uniform use of the GEP value (that is why I mentioned it in my version of the comment).  Uniform only states that for the *vectorized* value we can use the lane 0 scalar value.



Repository:
  rL LLVM

https://reviews.llvm.org/D24557