[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