[PATCH] D20315: [LV] For some induction variables, use vector phis instead of widening the scalar in the loop body

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

Thanks, Elena!


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:427
@@ +426,3 @@
+  /// Currently only works for integer primary induction variables with
+  /// a constant (non-SCEV) step.
+  /// If TruncType is provided, instead of widening the original IV, we
----------------
delena wrote:
> step is always a SCEV now. You mean step which is a constant SCEV.
Right, thanks!

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2118
@@ +2117,3 @@
+  ConstantInt *Step = II.getConstIntStepValue();
+
+  // Construct the initial value of the vector IV in the vector loop preheader
----------------
delena wrote:
> Check (Step != 0) here.
There's already an assert that this is non-null before each callsite. Do you want another assert here?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4110
@@ +4109,3 @@
+      }
+      Value *Broadcasted = getBroadcastInstrs(V);
+      // After broadcasting the induction variable we need to make the vector
----------------
delena wrote:
> do we need this code if VF==1?
We certainly need the loop below the getBroadcastInstrs call (note that loops until UF, not VF).
As to getBroadcastInstrs itself - InnerLoopUnroller::getBarocastInstrs() is a nop, which exists, I think, precisely so that we don't need to special-case VF==1.


================
Comment at: test/Transforms/LoopVectorize/X86/gather_scatter.ll:98
@@ -97,3 +97,3 @@
 ;AVX512-LABEL: @foo2
-;AVX512: getelementptr %struct.In, %struct.In* %in, <16 x i64> %induction, i32 1
+;AVX512: getelementptr %struct.In, %struct.In* %in, <16 x i64> %vec.ind, i32 1
 ;AVX512: llvm.masked.gather.v16f32
----------------
delena wrote:
> I propose to remove variable name from this test.
Sure.


http://reviews.llvm.org/D20315