GlobalISel: Relax verification of physical register copy types

This was picking a concrete size for a physical register, and
enforcing exact match on the virtual register's type size. Some
targets add multiple types to a register class, and some are smaller
than the full bit width. For example x86 adds f32 to 128-bit xmm
registers, and AMDGPU adds i16/f16 to 32-bit registers.

It might be better to represent these cases as a copy of the full
register and an extraction of the subpart, but a lot of code assumes
you can directly copy. This will help fix the current usage of the DAG
calling convention infrastructure which is incompatible with how
GlobalISel is now using it.

The API is somewhat cumbersome here, but I just mirrored the existing
functions, except now with LLTs (and allow returning null on failure,
unlike the MVT version). I think the concept of selecting register
classes based on type is flawed to begin with, but I'm trying to keep
this compatible with the existing handling.

Author: arsenm

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

@@ -301,6 +301,19 @@ class TargetRegisterInfo : public MCRegisterInfo {
     return false;
   }
 
+  /// Return true if the given TargetRegisterClass is compatible with LLT T.
+  bool isTypeLegalForClass(const TargetRegisterClass &RC, LLT T) const {
+    for (auto I = legalclasstypes_begin(RC); *I != MVT::Other; ++I) {
+      MVT VT(*I);
+      if (VT == MVT::Untyped)
+        return true;
+
+      if (LLT(VT) == T)
+        return true;
+    }
+    return false;
+  }
+
   /// Loop over all of the value types that can be represented by values
   /// in the given register class.
   vt_iterator legalclasstypes_begin(const TargetRegisterClass &RC) const {
@@ -320,6 +333,13 @@ class TargetRegisterInfo : public MCRegisterInfo {
   const TargetRegisterClass *getMinimalPhysRegClass(MCRegister Reg,
                                                     MVT VT = MVT::Other) const;
 
+  /// Returns the Register Class of a physical register of the given type,
+  /// picking the most sub register class of the right type that contains this
+  /// physreg. If there is no register class compatible with the given type,
+  /// returns nullptr.
+  const TargetRegisterClass *getMinimalPhysRegClassLLT(MCRegister Reg,
+                                                       LLT Ty = LLT()) const;
+
   /// Return the maximal subclass of the given register class that is
   /// allocatable or NULL.
   const TargetRegisterClass *

llvm/lib/CodeGen/MachineVerifier.cpp

@@ -1678,28 +1678,54 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
   case TargetOpcode::COPY: {
     const MachineOperand &DstOp = MI->getOperand(0);
     const MachineOperand &SrcOp = MI->getOperand(1);
-    LLT DstTy = MRI->getType(DstOp.getReg());
-    LLT SrcTy = MRI->getType(SrcOp.getReg());
+    const Register SrcReg = SrcOp.getReg();
+    const Register DstReg = DstOp.getReg();
+
+    LLT DstTy = MRI->getType(DstReg);
+    LLT SrcTy = MRI->getType(SrcReg);
     if (SrcTy.isValid() && DstTy.isValid()) {
       // If both types are valid, check that the types are the same.
       if (SrcTy != DstTy) {
         report("Copy Instruction is illegal with mismatching types", MI);
         errs() << "Def = " << DstTy << ", Src = " << SrcTy << "\n";
       }
+
+      break;
     }
-    if (SrcTy.isValid() || DstTy.isValid()) {
-      // If one of them have valid types, let's just check they have the same
-      // size.
-      unsigned SrcSize = TRI->getRegSizeInBits(SrcOp.getReg(), *MRI);
-      unsigned DstSize = TRI->getRegSizeInBits(DstOp.getReg(), *MRI);
-      assert(SrcSize && "Expecting size here");
-      assert(DstSize && "Expecting size here");
-      if (SrcSize != DstSize)
-        if (!DstOp.getSubReg() && !SrcOp.getSubReg()) {
-          report("Copy Instruction is illegal with mismatching sizes", MI);
-          errs() << "Def Size = " << DstSize << ", Src Size = " << SrcSize
-                 << "\n";
-        }
+
+    if (!SrcTy.isValid() && !DstTy.isValid())
+      break;
+
+    // If we have only one valid type, this is likely a copy between a virtual
+    // and physical register.
+    unsigned SrcSize = 0;
+    unsigned DstSize = 0;
+    if (SrcReg.isPhysical() && DstTy.isValid()) {
+      const TargetRegisterClass *SrcRC =
+          TRI->getMinimalPhysRegClassLLT(SrcReg, DstTy);
+      if (SrcRC)
+        SrcSize = TRI->getRegSizeInBits(*SrcRC);
+    }
+
+    if (SrcSize == 0)
+      SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
+
+    if (DstReg.isPhysical() && SrcTy.isValid()) {
+      const TargetRegisterClass *DstRC =
+          TRI->getMinimalPhysRegClassLLT(DstReg, SrcTy);
+      if (DstRC)
+        DstSize = TRI->getRegSizeInBits(*DstRC);
+    }
+
+    if (DstSize == 0)
+      DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
+
+    if (SrcSize != 0 && DstSize != 0 && SrcSize != DstSize) {
+      if (!DstOp.getSubReg() && !SrcOp.getSubReg()) {
+        report("Copy Instruction is illegal with mismatching sizes", MI);
+        errs() << "Def Size = " << DstSize << ", Src Size = " << SrcSize
+               << "\n";
+      }
     }
     break;
   }

llvm/lib/CodeGen/TargetRegisterInfo.cpp

@@ -225,6 +225,23 @@ TargetRegisterInfo::getMinimalPhysRegClass(MCRegister reg, MVT VT) const {
   return BestRC;
 }
 
+const TargetRegisterClass *
+TargetRegisterInfo::getMinimalPhysRegClassLLT(MCRegister reg, LLT Ty) const {
+  assert(Register::isPhysicalRegister(reg) &&
+         "reg must be a physical register");
+
+  // Pick the most sub register class of the right type that contains
+  // this physreg.
+  const TargetRegisterClass *BestRC = nullptr;
+  for (const TargetRegisterClass *RC : regclasses()) {
+    if ((!Ty.isValid() || isTypeLegalForClass(*RC, Ty)) && RC->contains(reg) &&
+        (!BestRC || BestRC->hasSubClass(RC)))
+      BestRC = RC;
+  }
+
+  return BestRC;
+}
+
 /// getAllocatableSetForRC - Toggle the bits that represent allocatable
 /// registers for the specific register class.
 static void getAllocatableSetForRC(const MachineFunction &MF,

llvm/test/MachineVerifier/test_copy_physregs_x86.mir

@@ -0,0 +1,54 @@
+#RUN: not --crash llc -march=x86-64 -run-pass=none -verify-machineinstrs -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: x86-registered-target
+
+# These copies have mismatched type sizes that are allowed because the
+# register class is defined to directly include the narrower type.
+---
+name:            test_valid_copies
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $xmm0, $xmm1, $xmm2, $xmm3, $xmm4
+    %0:_(s32) = COPY $xmm0
+    %1:_(s64) = COPY $xmm1
+    %2:_(s128) = COPY $xmm2
+    %3:_(<4 x s32>) = COPY $xmm3
+    %4:_(<2 x s64>) = COPY $xmm4
+    $xmm0 = COPY %0
+    $xmm1 = COPY %1
+    $xmm2 = COPY %2
+    $xmm3 = COPY %3
+    $xmm4 = COPY %4
+...
+
+---
+name:            test_invalid_copies
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $xmm0, $xmm1, $xmm2, $xmm3
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    %0:_(s16) = COPY $xmm0
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    %1:_(<4 x s16>) = COPY $xmm1
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    %2:_(s256) = COPY $xmm2
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    %3:_(<8 x s32>) = COPY $xmm3
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    $xmm0 = COPY %0
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    $xmm1 = COPY %1
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    $xmm2 = COPY %2
+
+    ; CHECK: *** Bad machine code: Copy Instruction is illegal with mismatching sizes ***
+    $xmm3 = COPY %3
+...