[MachineVerifier] Diagnose invalid INSERT_SUBREGs

[jroelofs]

hm. This breaks: llvm/test/CodeGen/X86/domain-reassignment.mir

%9:gr32 = INSERT_SUBREG %8:gr32(tied-def 0), %17:gr16, %subreg.sub_8bit_hi_phony

[jroelofs]

Fix llvm/test/CodeGen/X86/domain-reassignment.mir to not rely on magic constants.

[arsenm]

llvm/lib/CodeGen/MachineVerifier.cpp

1786

This is already TRI tin the verifier class

llvm/test/MachineVerifier/test_insert_subreg.mir

2

-global-isel doesn't do anything with -run-pass

[thakis]

This seems to break tests: http://45.33.8.238/linux/51314/step_12.txt

Please take a look.

[jroelofs]

Reverted for the moment while I work out what broke.

Thanks thakis!

[jroelofs]

One of the two issues in the tests was easily fixable:
https://reviews.llvm.org/D106168

The other issue appears to be much trickier, as it stems from the ARM backend treating MPR regs as if they were ssub subregs. This causes a contradiction in sizes, since ssub's are 32 bits whereas MPR regs are fp16's.

[uabelho]

llvm/lib/CodeGen/MachineVerifier.cpp

1787

Don't you need to take operand 2's subreg into account here?

I'm seeing failures for cases where

MI->getOperand(2).getReg()

is indeed too big, but

MI->getOperand(2).getSubReg()

is specifying a subreg of a small enough size so there is no problem.

[uabelho]

llvm/lib/CodeGen/MachineVerifier.cpp

1787

This seems to work better for me:

unsigned InsertedSize;
if (unsigned SubIdx = MI->getOperand(2).getSubReg())
  InsertedSize = TRI->getSubRegIdxSize(SubIdx);
else
  InsertedSize =
    TRI->getRegSizeInBits(MI->getOperand(2).getReg(), *MRI);

[jroelofs]

llvm/lib/CodeGen/MachineVerifier.cpp

1787

Good call, thanks!

4de74a7c4da3516d0d9119452cc073b061716b5d

llvm/lib/CodeGen/MachineVerifier.cpp

for (unsigned Idx = 0; Idx < MI->getNumDefs(); Idx++) {
       if (UseOpIdx < FirstGCPtrIdx || UseOpIdx > LastGCPtrIdx) {
         report("STATEPOINT def tied to non-gc operand", MI);
         break;
       }
     }
 
     // TODO: verify we have properly encoded deopt arguments
   } break;
+  case TargetOpcode::INSERT_SUBREG: {
+    unsigned InsertedSize =
+        TRI->getRegSizeInBits(MI->getOperand(2).getReg(), *MRI);
+    unsigned SubRegSize = TRI->getSubRegIdxSize(MI->getOperand(3).getImm());
+    if (SubRegSize < InsertedSize) {
+      report("INSERT_SUBREG expected inserted value to have equal or lesser "
+             "size than the subreg it was inserted into", MI);
+      break;
+    }
+  } break;
   }
 }
 
 void
 MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
   const MachineInstr *MI = MO->getParent();
   const MCInstrDesc &MCID = MI->getDesc();
   unsigned NumDefs = MCID.getNumDefs();

llvm/test/CodeGen/X86/domain-reassignment.mir

bb.2.else:
     %1 = COPY %13.sub_8bit
 
   bb.3.exit:
 
     ; check PHI, IMPLICIT_DEF, and INSERT_SUBREG replacers.
 
     %2 = PHI %1, %bb.2, %0, %bb.1
     %17 = IMPLICIT_DEF
-    %16 = INSERT_SUBREG %17, %2, 1
+    %16 = INSERT_SUBREG %17, %2, %subreg.sub_8bit_hi
     %18 = COPY %16
     %19 = COPY %6
     %21 = IMPLICIT_DEF
     %20 = VMOVSSZrrk %19, killed %18, killed %21, %5
     %22 = COPY %20
     VMOVSSZmr %4, 1, $noreg, 0, $noreg, killed %22 :: (store (s32) into %ir.fptr)
     RET 0
 
bb.0:
     %13 = SHL8ri %12, 1, implicit-def dead $eflags
     %14 = NOT8r %13
     %15 = OR8rr %14, %12, implicit-def dead $eflags
     %16 = AND8rr %15, %13, implicit-def dead $eflags
     %17 = XOR8rr %16, %12, implicit-def dead $eflags
     %18 = ADD8rr %17, %14, implicit-def dead $eflags
 
     %8 = IMPLICIT_DEF
-    %9 = INSERT_SUBREG %8, %18, 1
+    %9 = INSERT_SUBREG %8, %18, %subreg.sub_8bit_hi
     %10 = COPY %9
     %11 = VMOVAPDZrrk %2, killed %10, %1
     VMOVAPDZmr %0, 1, $noreg, 0, $noreg, killed %11
 
     ; FIXME We can't replace TEST with KTEST due to flag differences
     ; TEST8rr %18, %18, implicit-def $eflags
     ; JCC_1 %bb.1, 4, implicit $eflags
     ; JMP_1 %bb.2
bb.0:
     %12 = SHR16ri %7, 2, implicit-def dead $eflags
     %13 = SHL16ri %12, 1, implicit-def dead $eflags
     %14 = NOT16r %13
     %15 = OR16rr %14, %12, implicit-def dead $eflags
     %16 = AND16rr %15, %13, implicit-def dead $eflags
     %17 = XOR16rr %16, %12, implicit-def dead $eflags
 
     %8 = IMPLICIT_DEF
-    %9 = INSERT_SUBREG %8, %17, 3
+    %9 = INSERT_SUBREG %8, %17, %subreg.sub_16bit
     %10 = COPY %9
     %11 = VMOVAPSZrrk %2, killed %10, %1
     VMOVAPSZmr %0, 1, $noreg, 0, $noreg, killed %11
 
     ; FIXME We can't replace TEST with KTEST due to flag differences
     ; FIXME TEST16rr %17, %17, implicit-def $eflags
     ; FIXME JCC_1 %bb.1, 4, implicit $eflags
     ; FIXME JMP_1 %bb.2

llvm/test/MachineVerifier/test_insert_subreg.mir

+#RUN: not --crash llc -march=aarch64 -o - -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
+
+---
+name:            test_insert_subreg
+legalized:       true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+liveins:
+body:             |
+  bb.0:
+    liveins: $s0, $h1
+
+    %0:fpr32 = COPY $s0
+
+    ; FIXME: we can't diagnose this case because the ARM backend treats fp16
+    ; values as being ssub regs, creating a contradiction in the sizes.
+    ; CHECK-NOT: *** Bad machine code:
+    %1:fpr128 = IMPLICIT_DEF
+    %2:fpr128 = INSERT_SUBREG %1:fpr128, %0:fpr32, %subreg.hsub
+
+    ; CHECK: *** Bad machine code: INSERT_SUBREG expected inserted value to have equal or lesser size than the subreg it was inserted into ***
+    %3:fpr128 = IMPLICIT_DEF
+    %4:fpr128 = INSERT_SUBREG %3:fpr128, %0:fpr32, %subreg.dsub
+
+    ; CHECK-NOT: *** Bad machine code:
+    %7:fpr128 = IMPLICIT_DEF
+    %8:fpr128 = INSERT_SUBREG %7:fpr128, %0:fpr32, %subreg.ssub
+
+...