[PATCH] D88086: [ARM][MVE] tail-predication: checks for the elementcount, cont'd

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer updated this revision to Diff 293746.
SjoerdMeijer added a comment.

Thanks for looking Eli.

I have addressed the minor comment.

Regarding your comments about the overflow checks:  you are absolutely right about that these checks were not entirely right yet, which we discussed in a previous patch. I forgot to add a FIXME to the relevant code. What I have decided to do here is to just completely remove both checks `2.1` and `2.2` and put FIXMEs in there. Check `2.1` looking at it again, doesn't make sense, so don't see the point of keeping it. Check `2.2`, needs improvements, but then I decided to also get rid of it because it needs a rewrite (and so I don't see the point of keeping it). As we are save for now, the intrinsic is not exposed to the user yet, I thought to do the clean up here first, and address the improvements in a follow up.

To give an impression, I have briefly looked at the overflow checks again that I have removed here. What I am thinking about for the follow up is to approach the overflow checks in a different way because I just don't see how SCEV and integer ranges or the "is always positive" helpers are going to help me. My observation is that for most loops that we want to support, we have these patterns:

  tripcount: (1 + ((-4 + (4 * ((3 + %N) /u 4))<nuw>) /u 4))<nuw><nsw>
  elemcount: %N

or:

  tripcount: (1 + ((-4 + (4 * ({(3 + (sext i16 %Size to i32)),+,-1}<nw><%for.body> /u 4))<nuw>) /u 4))<nuw><nsw>
  elemcount: {(sext i16 %Size to i32),+,-1}<nw><%for.body>  

and write a SCEV visitor/matcher, and see if the elementcount is "part of the tripcount", i.e. if they are bound by the same variable modulo the rounding up, then we have a very strong indication that we are talking about a well behaving loop without any chance of overflow. So in a way it is similar to the code removed here in `Check 1` but used for another purpose I guess.


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

https://reviews.llvm.org/D88086

Files:
  llvm/lib/Target/ARM/MVETailPredication.cpp
  llvm/test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-basic.ll
  llvm/test/CodeGen/Thumb2/LowOverheadLoops/tail-pred-forced.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88086.293746.patch
Type: text/x-patch
Size: 12835 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200923/e52e692b/attachment-0001.bin>