[PATCH] D85737: ARM][MVE] tail-predication: overflow checks for backedge taken count

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:374
   auto *BTC = SE->getSCEV(BackedgeTakenCount);
+  auto *MaxBTC = SE->getConstantMaxBackedgeTakenCount(L);
+
----------------
SjoerdMeijer wrote:
> efriedma wrote:
> > I'm forgetting, do we have a check somewhere that BackedgeTakenCount is actually the backedge-taken count of the loop?
> Good point. I thought we had one, but we have similar checks for the IV, which is done in step 3) below on line 464. Will address this in a follow up.
I think I messed up reviewing this.  Took me a bit of time to remember what's going on here.

`SE->getConstantMaxBackedgeTakenCount()` is the backedge-taken count of the loop.  BTC is the number of elements the loop processes, minus one.  If you want to ensure BTC + 1 doesn't overflow, getConstantMaxBackedgeTakenCount() doesn't actually help, unless you prove some connection between the two values.  The code currently in IsSafeActiveMask does not try to prove that connection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85737