[PATCH] D21330: Loop vectorization with FP induction variables

[Elena Demikhovsky via llvm-commits]

delena added inline comments.

================
Comment at: ../include/llvm/Transforms/Utils/LoopUtils.h:328-329
@@ +327,4 @@
+
+  /// Returns binary opcode.
+  Instruction::BinaryOps getBinaryOpcode() const { return BinaryOp; }
+
----------------
anemet wrote:
> binary op -> induction op is better everywhere.   Also I am assuming this is the op that advances the induction variable.  You may want to spell this out somewhere.
Ok. Changing it to getInductionBinOp()

================
Comment at: ../lib/Transforms/Utils/LoopUtils.cpp:770
@@ +769,3 @@
+  if (TheLoop->getHeader() != Phi->getParent())
+    return nullptr;
+
----------------
anemet wrote:
> this function is returning a bool
Fixed. Thanks.

================
Comment at: ../lib/Transforms/Utils/LoopUtils.cpp:816-818
@@ +815,5 @@
+  // on function level.
+  Instruction *UAI = !BOp->hasUnsafeAlgebra() ? BOp : nullptr;
+  D = InductionDescriptor(StartValue, IK_FpInduction, Step, UAI,
+                          BOp->getOpcode());
+  return true;
----------------
anemet wrote:
> I think that a better interface would be to take BOp (step instruction) optionally and then derive DI::hasUnsafeAlgebra and the opcode from that.  This is OK as a follow-up if you prefer.
I did it at the beginning and then prefer to follow Reduction implementation, just to be consistent with existing interface.

================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:4104-4105
@@ +4103,4 @@
+    Value *Broadcasted = getBroadcastInstrs(V);
+    // After broadcasting the induction variable we need to make the vector
+    // consecutive by adding 0, 1, 2, etc.
+    Value *StepVal = cast<SCEVUnknown>(II.getStep())->getValue();
----------------
anemet wrote:
> by adding StepVal, you mean
I fixed the comment. thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D21330