[PATCH] D72629: [ARM][MVE] Disallow VPSEL for tail predication

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Sounds good. I think that if we wanted to make VPSEL work, it would be better to try to convert them to VMOVt in a VPT block. (Although if there is only one of them, creating the block maybe more overhead. Hopefully the VCMP can be folded in).

Does VPNOT not need to be explicitly excluded because they are def's anyway? Might be worth leaving a comment to explain that to others (or us in the future). We might also want to say any instruction with an Else predicate (not a Then predicate) would be invalid too. But we don't generate those anywhere yet, so that can be separate.

There are a lot of large tests and it's hard to see what has changed and what hasn't with the patch. Can you add a quick comment to each that says what it's trying to test? And maybe if the MIR is modified. inloop-vpsel-1.mir and inloop-vpsel-3.mir seem to be the same.



================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/inloop-vpnot-2.mir:224
+    MVE_VPST 4, implicit $vpr
+    renamable $vpr = MVE_VPNOT renamable $vpr, 0, killed renamable $vpr
+    renamable $r5 = MVE_VSTRWU32_post renamable $q0, killed renamable $r5, 16, 1, killed renamable $vpr :: (store 16 into %ir.lsr.cast.e, align 4)
----------------
This is a vpnot inside an VPT block? Crazy

This wasn't code-gen'd right? I don't think VPNOT of VPSEL should end up inside a VPT block at present. They are free-standing. (And VPNOT should turn the following instructions into "Else's", not "Then's".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72629