[PATCH] D142875: [LV] Predicated epilog vectorization

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Hi @dmgreen, thanks for this patch - it adds some very useful functionality to the vectoriser and allows us to reduce the code size for epilogues too! I just had a few questions ...



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5653
 
-  if (!isEpilogueVectorizationProfitable(MainLoopVF)) {
+  if (!isScalarEpilogueAllowed()) {
+    // If we can fold the tail by masking to produce a predicated epilog,
----------------
Hi @dmgreen, to be honest this logic feels really counter-intuitive to me. At least on the surface, it seems to be saying if we explicitly do not want an epilogue, then ignore it and generate an epilogue. Do you know under which circumstances we would even reach this code?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5694
   for (auto &NextVF : ProfitableVFs)
-    if (((!NextVF.Scheme.Width.isScalable() && MainLoopVF.isScalable() &&
+    if (((!NextVF.Scheme.Width.isScalable() && MainLoopVF.Width.isScalable() &&
           ElementCount::isKnownLT(NextVF.Scheme.Width, EstimatedRuntimeVF)) ||
----------------
I assume we also reach this point with tail-folded schemes and choose the lowest cost out of all the tail-folded and normal vectorized plans?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10665
       VectorizationFactor EpilogueVF =
-          CM.selectEpilogueVectorizationFactor(VF.Scheme.Width, LVP);
-      if (EpilogueVF.Scheme.Width.isVector()) {
-
+          CM.selectEpilogueVectorizationFactor(VF.Scheme, LVP);
+      if (!VF.Scheme.FoldTailByMasking && EpilogueVF.Scheme.Width.isVector()) {
----------------
I think it's better if we can avoid calling this if we don't need to. We may go to a lot of effort to choose a suitable epilogue VF, looking through all the plans, only to just ignore the result completely if we're already tail-folding.


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

https://reviews.llvm.org/D142875