[PATCH] D18537: Don't vectorize loops when everything will be scalarized

[Adam Nemet via llvm-commits]

anemet accepted this revision.
anemet added a comment.
This revision is now accepted and ready to land.

LGTM.

I have two suggestions you may want to consider.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1535-1539
@@ -1534,6 +1534,7 @@
 private:
   /// Returns the expected execution cost. The unit of the cost does
   /// not matter because we use the 'cost' units to compare different
   /// vector widths. The cost that is returned is *not* normalized by
   /// the factor width.
-  unsigned expectedCost(unsigned VF);
+  unsigned expectedCost(unsigned VF, bool &ActuallyVectorized);
+
----------------
Assuming I understand the logic correctly, you may want to mention it here that for ActuallyVectorized we currently use type legality.   (I.e. we could still have a loop where we don't vectorize anything because none of the *instructions* are legal.)

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5557-5558
@@ -5543,3 +5556,4 @@
 
-unsigned LoopVectorizationCostModel::expectedCost(unsigned VF) {
+unsigned LoopVectorizationCostModel::expectedCost(unsigned VF,
+                                                  bool &ActuallyVectorized) {
   unsigned Cost = 0;
----------------
It seems that the current logic requires ActuallyVectorized to be initialized to false by the caller.  I think that it would less error-prone if you initialized to false here inside the API.

What do you think?


http://reviews.llvm.org/D18537