[PATCH] D21620: [LV] Don't widen trivial induction variables
Michael Kuperstein via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 23 11:45:20 PDT 2016
mkuper added a comment.
Basically this LGTM (modulo some nits, inline), as long as you also file a bug for the InstCombine issue.
I'm also ok with landing only the scalar IV part of this for now, and trying to make a decision about getScalarizedStepVector vs. IC separately. Adam, what do you think?
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4223
@@ +4222,3 @@
+ // vector to give InstCombine a better chance of simplifying it.
+ if (VF > 1 && ValuesToIgnore->count(P)) {
+ for (unsigned part = 0; part < UF; ++part)
----------------
Nit - can you perhaps rename ValuesToIgnore? Because we're not using it here to "ignore" anything, and it looks odd.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6352
@@ -6275,1 +6351,3 @@
+ return U == PN || isa<ICmpInst>(U) ||
+ isa<GetElementPtrInst>(U);
})) {
----------------
Note that there's a conflicting modification to this code planned by Wei: http://reviews.llvm.org/D20474
================
Comment at: test/Transforms/LoopVectorize/induction.ll:110
@@ +109,3 @@
+; Make sure we scalarize the step vectors used for the induction variables. We
+; can't easily simply vectorized step vectors.
+;
----------------
simply -> simplify
http://reviews.llvm.org/D21620
More information about the llvm-commits
mailing list