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

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:382
   // TODO: This add likely already exists in the loop.
   Value *Remaining = Builder.CreateSub(Processed, Factor);
   Processed->addIncoming(Remaining, L->getLoopLatch());
----------------
efriedma wrote:
> efriedma wrote:
> > samparker wrote:
> > > efriedma wrote:
> > > > This subtraction can also overflow.
> > > But that's okay, right? This predication is only really useful when wrapping happens and the intrinsic reflects that overflow can/will happen.
> > The problem here is that the original get_active_lane_mask can do something like this:
> > 
> > Iteration 1: get_active_lane_mask(0, 5) -> all-true
> > Iteration 2: get_active_lane_mask(4, 5) -> <true, false, false, false>
> > Iteration 3: get_active_lane_mask(8, 5) -> all-false
> > 
> > In the rewritten code, you end up with this:
> > 
> > Iteration 1: vctp(5) -> all-true
> > Iteration 2: vctp(1)- > <true, false, false, false>
> > Iteration 3: vctp(-3)- > all-true
> > 
> > There are a couple ways you could deal with this:
> > 
> > 1. Use saturating subtraction
> > 2. Prove the trip count is small enough that this never happens, i.e. `ceil(ElementCount / VectorWidth) >= TripCount`.
> Using saturating subtraction would require proving that the induction variable used as the first argument to llvm.get.active.mask doesn't overflow.  But I guess you have to check that anyway.
I would be hesitate to introduce saturating math here. I think the reasonable assumption is that the sub will wrap, but only on the final iteration. So just asserting that the element count is within the bounds of the trip count should be fine. 


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

https://reviews.llvm.org/D79175