[PATCH] D30211: [LV] Merge floating point and integer induction widening code

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

Thanks, this basically looks good, a few comments inline.



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:387
 
+/// A helper function that adds a 'fast' flag to floating point operations.
+static Value *addFastMathFlag(Value *V) {
----------------
Nothing changed here, you just moved it around, right?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:400
+static Constant *getIntOrFpConstant(Type *Ty, unsigned C) {
+  return Ty->isIntegerTy() ? ConstantInt::get(Ty, C) : ConstantFP::get(Ty, C);
+}
----------------
One of the users of this for the int case was a getSigned(), and now it's a get(). Are you sure this is correct?


================
Comment at: test/Transforms/LoopVectorize/float-induction.ll:4
 ; RUN: opt < %s  -loop-vectorize -force-vector-interleave=2 -force-vector-width=1 -dce -instcombine -S | FileCheck --check-prefix VEC1_INTERL2 %s
+; RUN: opt < %s  -loop-vectorize -force-vector-interleave=1 -force-vector-width=2 -dce -simplifycfg -instcombine -S | FileCheck --check-prefix=VEC2_INTERL1_PRED_STORE %s
 
----------------
Did you run it through the update script? If you did, could you have the diff show the actual diff vs. running it with the old code?


https://reviews.llvm.org/D30211