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

[Matthew Simpson via llvm-commits]

mssimpso 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);
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D24557