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

[Eli Friedman via Phabricator via llvm-commits]

efriedma 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,
----------------
Can `ElementCount + (VW-1)` overflow?  Do we need to check for that?


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:483
+  ConstantRange Diff = R1.difference(R2);
+  if (!Diff.isEmptySet()) {
+    const APInt *D = Diff.getSingleElement();
----------------
The general idea here makes sense.  The precise way you're implementing it seems a little strange; it's fine if TripCount is smaller than BTC, I think.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:516
+  if (VectorWidth == StepValue ||
+      VectorWidth == -StepValue)
+  return true;
----------------
Is it really legal for the induction variable to be stepping in either direction?


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:537
+  for (auto &I : *Preheader) {
+    if (&I == BTC) {
+      LLVM_DEBUG(dbgs() << "ARM TP: Found num elems: " << I << "\n");
----------------
Not sure we can safely assume "I" is an add instruction.


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

https://reviews.llvm.org/D79175