[PATCH] D65197: [LV] Tail-loop Folding

[Hideki Saito via Phabricator via llvm-commits]

hsaito added a comment.

We probably need to discuss whether vectorize_predicate(enable) should (or should not) implicitly turns on vectorize(enable) or not. I guess the current behavior is "does not", right? We don't have to discuss that in this review, but we still want to make a conscious decision one way or the other, or did I miss that discussion?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:845
+  CM_ScalarEpilogueNotAllowedLowTripLoop,
+  CM_ScalarEpilogueNotAllowedPredicatePragma
 };
----------------
I think the nuance here is rather ScalarEpilogueNotNeededPredicatePragma. In other words, if scalar epilogue is needed for some other reason (but still okay to skip scalar epilogue execution when vector code executes), scalar epilogue can be emitted/utilized. Runtime vectorization legality check of all kinds fits in that profile. We shouldn't overload "predicated vector code" pragma with "don't emit scalar epilogue" meaning.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4765
+    break;
+  case CM_ScalarEpilogueNotAllowedLowTripLoop:
+    LLVM_DEBUG(dbgs() << "LV: Not allowing scalar epilogue due to low trip "
----------------
-Os/-Oz message comes out from fall through. Not desired.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4784
 
+  // Bail if we don't have a tail at all.
   unsigned MaxVF = computeFeasibleMaxVF(TC);
----------------
How about
  // Accept MaxVF if we don't have a tail at all.
and move the comment inside IF.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65197/new/

https://reviews.llvm.org/D65197