[PATCH] D72509: [ARM][LowOverheadLoops] Allow all MVE instrs.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

This looks sensible, using the VPT blocks to know that the instructions are predicated on something that makes tail predication valid.

I had couple of questions about some of the existing details. The VPSEL question is the only one really relevant here, the others are probably best looked at elsewhere (or they may be handled already in here somewhere, or I may just be thinking about them wrongly). They shouldn't block this improvement.



================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:525
+  if ((Flags & ARMII::DomainMask) != ARMII::DomainMVE)
+    return true;
+
----------------
What if we had something like a `VMOV s0 s1`. I know that's not anything to do with this patch, but is that outlawed anywhere at the moment?


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:530
+  // VPT block.
+  if ((Flags & ARMII::ValidForTailPredication) == 0 && !IsUse) {
+    LLVM_DEBUG(dbgs() << "ARM Loops: Can't tail predicate: " << *MI);
----------------
What about VPNOT or VPSEL? They would use both the vpr and the tail predicate to different extents.

VPSEL we seem to mark as IsValidForTailPredication, which I'm not sure about. We need to make sure it's using the "same" predicate as before (and making sure it doesn't just use whatever we had last put there, if we delete the vcpt!)




================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/vmldava_in_vpt.mir:174
+  ; CHECK:   renamable $q3 = MVE_VMINu32 renamable $q2, renamable $q0, 0, $noreg, undef renamable $q3
+  ; CHECK:   renamable $r12 = MVE_VMLADAVas32 killed renamable $r12, killed renamable $q3, killed renamable $q2, 0, killed $noreg
+  ; CHECK:   $lr = MVE_LETP killed renamable $lr, %bb.2
----------------
Another unrelated one, but there is nothing in the instructions of this loop that says that this is predicated on a hardware register, right? Or that is has unmodelled side effects? The refs to vpr and the vctp are removed but no other ref is added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72509