[PATCH] D79783: [LoopVectorize] Fallback to a scalar epilogue when TP fails

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D79783#2044036 <https://reviews.llvm.org/D79783#2044036>, @Pierre-vh wrote:

> - Addressed comments (see items marked as "done")
> - Changed "reportVectorizationFailure" calls in "prepareTailFoldingByMasking" into simple debug prints.
>
> > Would be good to also preserve the existing "force-vector-tail-folding" behavior of "fold or do not vectorize", in addition to introducing the new "prefer-vector-tail-folding" behavior.
>
> What is your preferred solution for this? Here are some ideas:
>
> - Add a new command-line switch that acts like the old `-prefer-predicate-over-epilog`, something like `-force-predicate-over-epilog`
> - Add an additional switch to use with `-prefer-predicate-over-epilog` to disable the fallback behaviour, such as `-disable-scalar-epilog-fallback`
> - Keep the old `-prefer-predicate-over-epilog` behaviour and only fallback when TTI requested tail-folding


Perhaps either (the first option of) a new command-line switch e.g. `-force-vector-tail-folding`, or a value added to `-prefer-predicate-over-epilog=x` indicating the "strength" of the preference.
Adding a matching pragma/metadata would also be nice.
Better avoid adding a new TTI hook, which will not be exercised by any in-tree target, right?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1264
+  SmallPtrSet<const Instruction *, 8> TmpMaskedOp;
+  SmallPtrSet<Instruction *, 8> TmpConditionalAssumes;
+
----------------
Can add a comment what these sets are for.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1257
+            "LiveOutFoldingTailByMasking", ORE, TheLoop, UI);
+      } else {
+        LLVM_DEBUG(
----------------
Ayal wrote:
> Should an "analysis" message be reported instead of a "failure" one?
Would be good to also have an ORE report similar to the original one, except as an analysis rather than failure.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5027
+                         "hint/switch and using a scalar epilogue instead.\n");
+    ScalarEpilogueStatus = CM_ScalarEpilogueAllowed;
+    return MaxVF;
----------------
Ayal wrote:
> Why change ScalarEpilogueStatus?
Why change ScalarEpilogueStatus?


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

https://reviews.llvm.org/D79783