[PATCH] D86074: [ARM][MVE] Tail-predication: check get.active.lane.mask's TC value

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:376
 
-  // 1) TODO: Check that the TripCount (TC) belongs to this loop (originally).
+  // 1) Check that the original scalar loop TripCount (TC) belongs to this loop.
   // The scalar tripcount corresponds the number of elements processed by the
----------------
SjoerdMeijer wrote:
> efriedma wrote:
> > Why do we need this check?  Emitting vctp32 should be okay even if we can't actually tail-predicate the loop.  The overflow check should be enough to ensure that's it's safe to emit vctp32, I think?  Or am I forgetting somthing?
> I could have a look where exactly, but if I am not mistaken you suggested on of the previous patches that we need to check that this tripcount/elementcount actually belongs to this loop. similarly like we already did for the IV. The reasoning was that for now get.active.lane.mask is emitted from the vector for nicely behaving loops, but it wouldn't be difficult to imagine that soon we will have a corresponding user-facing intrinsic. I think I am quoting that, if I remember well, and so these checks are needed.
> 
> And if we emit the VCTP, then that represents tail-predication. I.e., the VCTP intrinsic can be picked up in the LoweroverheadLoop pass and turned into a tail-predicated loop (after additional checks).
I did have a look because I was curious if I had starting imaging things. This is the remark I had in mind:

https://reviews.llvm.org/D79175#2063586

This is remark is explicitly about "L" though. And I thought there was a similar remark about the 2nd argument when it still was the BTC (previous version of this patch), but I don't think I can't find that easily now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86074