[PATCH] D85980: [ARM][LV] Add a preferPredicatedReductionSelect target hook

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

nit about the subject line:

> [ARM][LV] Add a preferPredicatedReductionSelect target hook

[LV] - > [TTI]?



================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1591
 
-  // If there are live-out values, it is probably a reduction, which needs a
-  // final reduction step after the loop. MVE has a VADDV instruction to reduce
-  // integer vectors, but doesn't have an equivalent one for float vectors. A
-  // live-out value that is not recognised as a reduction will result in the
-  // tail-predicated loop to be reverted to a non-predicated loop and this is
-  // very expensive, i.e. it has a significant performance impact. So, in this
-  // case it's better not to tail-predicate the loop, which is what we check
-  // here. Thus, we allow only 1 live-out value, which has to be an integer
-  // reduction, which matches the loops supported by ARMLowOverheadLoops.
-  // It is important to keep ARMLowOverheadLoops and canTailPredicateLoop in
-  // sync with each other.
+  // If there are live-out values, it is probably a reduction. We can predicate
+  // most reduction operations freely under MVE using a combination of
----------------
Not sure if this also needs to be split off from the TTI part, so that we have 3 parts: LV, TTI, and the ARM change. But I am fine with this, and will leave this up to you whatever you think is most convenient (you can also just commit the TTI part, then this change).


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1606
+    if (!I->getType()->isIntegerTy() && !I->getType()->isFloatTy() &&
+        !I->getType()->isHalfTy()) {
+      LLVM_DEBUG(dbgs() << "Don't tail-predicate loop with non-integer/float "
----------------
hey, that's new, I forgot but do we have a (negative) test for half types? :-)


================
Comment at: llvm/test/Transforms/LoopVectorize/ARM/tail-folding-not-allowed.ll:269
 
-; Don't tail-fold float reductions.
-;
----------------
I recently reorganised all the tail folding test (because it was getting a bit of a mess).
Looks like `tail-folding-allowed.ll` would now be a better home for this test (and the others below)?


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

https://reviews.llvm.org/D85980