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

[Hideki Saito via Phabricator via llvm-commits]

hsaito added a comment.

In D65197#1604940 <https://reviews.llvm.org/D65197#1604940>, @SjoerdMeijer wrote:

> Friendly ping :-)


Looks like we are converging. One minor comment only.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:845
+  CM_ScalarEpilogueNotAllowedLowTripLoop,
+  CM_ScalarEpilogueNotAllowedPredicatePragma
 };
----------------
hsaito wrote:
> 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.
Thanks for addressing. May not be immediately effective, but should help if someone wants to move towards that direction.


================
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 "
----------------
hsaito wrote:
> -Os/-Oz message comes out from fall through. Not desired.
Thanks for taking care of it.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4804
 
+  // Accept MaxVF if we do not have a tail.
   unsigned MaxVF = computeFeasibleMaxVF(TC);
----------------
Suggest moving this comment between the Lines 4806 and 4807.


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

https://reviews.llvm.org/D65197