[PATCH] D21620: [LV] Don't widen trivial induction variables

[Matthew Simpson via llvm-commits]

mssimpso added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2191
@@ +2190,3 @@
+
+  // We can't create a vector with less than two elements.
+  assert(VF > 1 && "VF should be greater than one");
----------------
anemet wrote:
> Matt, your argument about instcombine not being adequate for this is a convincing one.
> 
> On the other hand, I am still wondering if the solution is general enough.  There should be cases where we'd be better off having *both* the vector and the scalar version of the same induction variable.  I.e. depending on the use, you may want to use the vector version (e.g. store) or the scalar version (address calc, compares).
> 
> I am not saying that we need to necessarily implement this but the current solution should be taking us incrementally closer to the full solution.
Adam,

One way to handle the cases you mention would be to allow the vectorizer to generate both a scalar version and a vector version of the same induction variable. Then it could select whether to generate a use of the scalar or vector version based on the instruction (address calculation, store, etc.). DCE and Instcombine would presumably clean up afterwards.

The current patch does take us incrementally to that kind of solution. It handles the case where we know we will always prefer to use the scalar version. If that's the case, we generate the scalar steps; otherwise, we continue to generate the vector steps. It doesn't, however, generate the scalar version of the induction variable along side the vector version.


http://reviews.llvm.org/D21620