[PATCH] D21330: Loop vectorization with FP induction variables

[Elena Demikhovsky via llvm-commits]

delena marked an inline comment as done.

================
Comment at: ../lib/Analysis/ScalarEvolutionExpander.cpp:1614
@@ -1613,3 +1613,3 @@
   Value *V = expand(SH);
-  if (Ty) {
+  if (Ty && SE.isSCEVable(Ty)) {
     assert(SE.getTypeSizeInBits(Ty) == SE.getTypeSizeInBits(SH->getType()) &&
----------------
sanjoy wrote:
> This is odd -- is it just to help keep the `Step` as a `SCEV *`?  If so, I'd suggest solving that within `InductionDescriptor` itself (i.e. maybe support having the step as either a `SCEV *` or a `Value *`, depending on the type of the `InductionDescriptor`?).
I't dropping this change, I don't need it anymore.

================
Comment at: ../lib/Transforms/Utils/LoopUtils.cpp:787
@@ +786,3 @@
+
+  if (!isa<BinaryOperator>(BEValue))
+    return false;
----------------
sanjoy wrote:
> Maybe use a `dyn_cast` here?
Ok.

================
Comment at: ../lib/Transforms/Utils/LoopUtils.cpp:811
@@ +810,3 @@
+
+  Instruction *UAI = !BOp->hasUnsafeAlgebra() ? BOp : nullptr;
+  D = InductionDescriptor(StartValue, IK_FpInduction, Step, UAI,
----------------
sanjoy wrote:
> The condition looks inverted?
The hasUnsafeAlgebra() means that instruction itself has "fast" attribute. In this case we don't need additional check. But if the BOp does not have the "fast" attribute, the legality of FP transformation should be allowed on  function  level. I'll add a comment.


Repository:
  rL LLVM

https://reviews.llvm.org/D21330