[PATCH] D21330: Loop vectorization with FP induction variables

[Adam Nemet via llvm-commits]

anemet added a comment.

In https://reviews.llvm.org/D21330#488606, @mkuper wrote:

> LGTM


I've just started looking at this too.  Please give me a few mins.  So far I only encountered minor things.


================
Comment at: ../include/llvm/Transforms/Utils/LoopUtils.h:308
@@ +307,3 @@
+  /// the data describing this induction.
+  static bool isFpInductionPHI(PHINode *Phi, const Loop* L,
+                               ScalarEvolution *SE, InductionDescriptor &D);
----------------
Fp->FP

================
Comment at: ../include/llvm/Transforms/Utils/LoopUtils.h:328-329
@@ +327,4 @@
+
+  /// Returns binary opcode.
+  Instruction::BinaryOps getBinaryOpcode() const { return BinaryOp; }
+
----------------
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.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D21330