[ARM] Remove kill flags between VCMP and insertion point

[dmgreen]

Summary

When we fold a VCMP into a VPST instruction any kill flags between the old VCMP position and the new insertion point need to be removed, in order to keep the verifier happy.

[samtebbs]

LGTM! Will we need to make similar change for the combination that happens in the LowOverheadLoops pass as well?

[dmgreen]

Thanks

Will we need to make similar change for the combination that happens in the LowOverheadLoops pass as well?

Yeah quite probably. This ran into a machine-verifier problem, which we should try and keep that happy if we can.

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

while (MBIter != EndIter) {
     LLVM_DEBUG(dbgs() << "  final block mask: " << (unsigned)BlockMask << "\n");
     if (MachineInstr *VCMP = findVCMPToFoldIntoVPST(MI, TRI, NewOpcode)) {
       LLVM_DEBUG(dbgs() << "  folding VCMP into VPST: "; VCMP->dump());
       MIBuilder = BuildMI(Block, MI, DL, TII->get(NewOpcode));
       MIBuilder.addImm((uint64_t)BlockMask);
       MIBuilder.add(VCMP->getOperand(1));
       MIBuilder.add(VCMP->getOperand(2));
       MIBuilder.add(VCMP->getOperand(3));
+
+      // We need to remove any kill flags between the original VCMP and the new
+      // insertion point.
+      for (MachineInstr &MI :
+           make_range(VCMP->getIterator(), MI->getIterator())) {
+        MI.clearRegisterKills(VCMP->getOperand(1).getReg(), TRI);
+        MI.clearRegisterKills(VCMP->getOperand(2).getReg(), TRI);
+      }
+
       VCMP->eraseFromParent();
     } else {
       MIBuilder = BuildMI(Block, MI, DL, TII->get(ARM::MVE_VPST));
       MIBuilder.addImm((uint64_t)BlockMask);
     }
 
     finalizeBundle(
         Block, MachineBasicBlock::instr_iterator(MIBuilder.getInstr()), MBIter);

llvm/test/CodeGen/Thumb2/mve-vpt-block-kill.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=thumbv8.1m.main-none-none-eabi -mattr=+mve.fp -run-pass arm-mve-vpt %s -o - -verify-machineinstrs | FileCheck %s
+
+# Check we remove kill flags when combining VCMP into a VPST
+
+--- |
+  define i32 @b() {
+    ret i32 undef
+  }
+
+...
+---
+name:            b
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $r0, $r1, $r4, $r6, $r7, $lr
+
+    ; CHECK-LABEL: name: b
+    ; CHECK: liveins: $r0, $r1, $r4, $r6, $r7, $lr
+    ; CHECK: $sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, killed $r4, killed $r6, killed $r7, killed $lr
+    ; CHECK: renamable $q0 = MVE_VMOVimmi32 0, 0, $noreg, undef renamable $q0
+    ; CHECK: renamable $q1 = MVE_VMOVimmi32 1, 0, $noreg, undef renamable $q1
+    ; CHECK: renamable $q2 = MVE_VADD_qr_i32 renamable $q1, renamable $r1, 0, $noreg, undef renamable $q2
+    ; CHECK: BUNDLE implicit-def dead $vpr, implicit-def $q0, implicit-def $d0, implicit-def $s0, implicit-def $s1, implicit-def $d1, implicit-def $s2, implicit-def $s3, implicit killed $q0, implicit $q1, implicit killed $q2 {
+    ; CHECK:   MVE_VPTv4u32 8, renamable $q0, renamable $q1, 8, implicit-def $vpr
+    ; CHECK:   renamable $q0 = MVE_VORR killed renamable $q0, killed renamable $q2, 1, internal killed renamable $vpr, renamable $q0
+    ; CHECK: }
+    ; CHECK: $sp = frame-destroy t2LDMIA_RET $sp, 14 /* CC::al */, $noreg, def $r4, def $r6, def $r7, def $pc, implicit undef $r0
+    $sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, killed $r4, killed $r6, killed $r7, killed $lr
+    renamable $q0 = MVE_VMOVimmi32 0, 0, $noreg, undef renamable $q0
+    renamable $q1 = MVE_VMOVimmi32 1, 0, $noreg, undef renamable $q1
+    renamable $vpr = MVE_VCMPu32 renamable $q0, renamable $q1, 8, 0, $noreg
+    renamable $q2 = MVE_VADD_qr_i32 killed renamable $q1, renamable $r1, 0, $noreg, undef renamable $q2
+    renamable $q0 = MVE_VORR killed renamable $q0, killed renamable $q2, 1, killed renamable $vpr, renamable $q0
+    $sp = frame-destroy t2LDMIA_RET $sp, 14 /* CC::al */, $noreg, def $r4, def $r6, def $r7, def $pc, implicit undef $r0
+
+...