[PATCH] D79175: [ARM][MVE] Tail-Predication: use @llvm.get.active.lane.mask to get the BTC

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer marked 2 inline comments as done.
SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:456
+  auto *ElementCount = SE->getAddExpr(BTC, One);
+  // Tmp = ElementCount + (VW-1)
+  auto *Tmp = SE->getAddExpr(ElementCount,
----------------
efriedma wrote:
> Can `ElementCount + (VW-1)` overflow?  Do we need to check for that?
We are not generating code for `ElementCount + (VW-1) `, so that one is fine.  We do want to know about overflow for `Ceil`, so will add a check for that.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:516
+  if (VectorWidth == StepValue ||
+      VectorWidth == -StepValue)
+  return true;
----------------
efriedma wrote:
> Is it really legal for the induction variable to be stepping in either direction?
It's definitely a case we want to support. In D77635, the vectoriser was taught to create a vector induction variable when a primary induction is initially absent, which is the case with decrementing loops, i.e. a step value of -1. We have a test case for counting down loops here  in `test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-const.ll` with function `@foo4`.

As this is produced by the vectoriser, I didn't see a problem with this, but will give it some more thoughts if we need to check more for this, but if it helps to get a first version in, I can remove this and address this in a follow up.


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

https://reviews.llvm.org/D79175