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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

The direction looks good to me. We definitely still want to vectorise if possible, if we don't do that when tail-folding is requested, that sounds like a "performance bug".

Just a general remark: tail-predication is a bit of a MVE term. In the LV, tail-folding is the term that is used. Please use that instead in the summary/description etc.

I had a first look, see some nits/questions inline.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1233
 
-bool LoopVectorizationLegality::prepareToFoldTailByMasking() {
+bool LoopVectorizationLegality::prepareToFoldTailByMasking(bool SoftFailure) {
 
----------------
nit: how about SoftFailure -> ReportFailure?
(and that would mean flipping the default value)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4972
+      LLVM_DEBUG(
+          dbgs() << "LV: Loop does not support tail-predication, ignoring "
+                    "hint/switch and falling back to a scalar epilogue.\n");
----------------
nit: tail-predication -> tail-folding


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5021
+    // Cleanup if we already planned to fold the tail.
+    if (FoldTailByMasking) {
+      Legal->abandonTailFoldingByMasking();
----------------
Do we need this if there is no tail? I haven't reminded myself and checked the flow, but can this condition be true?


================
Comment at: llvm/test/Transforms/LoopVectorize/ARM/tail-folding-scalar-epilogue-fallback.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -loop-vectorize -mattr=+armv8.1-m.main,+mve.fp -disable-mve-tail-predication=false < %s | FileCheck %s
+; RUN: opt -S -loop-vectorize -mattr=+armv8.1-m.main,+mve.fp -disable-mve-tail-predication=true < %s | FileCheck %s
----------------
Sorry for being a bit lazy, but this is a big example, but why is this rejected for tail-predication? Would be good to indicate the reason somewhere, e.g. function name, in the IR.
And could this example be reduced?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79783