[MachineVerifier] Add check that tied physregs aren't different.

[JesperAntonsson]

Summary

Added MachineVerifier code to check register ties more thoroughly, especially so that physical registers that are tied are the same. This may help e.g. when creating MIR files.

[JesperAntonsson]

Missed adding llvm-commits as subscriber initially. Adding this note as a ping to the mailing list.

[qcolombet]

lib/CodeGen/MachineVerifier.cpp

993 ↗

(On Diff #103184)

I'm not sure about that one.
Theoretically, you could have a not yet allocated vreg here. As long as the allocation does the right thing with the tie, that isn't an error.

test/CodeGen/MIR/X86/subregister-index-operands.mir

27 ↗

(On Diff #103184)

I would expect we add a new test instead of modifying this one.
In particular I don't think this one is going to fail when you modify the isPhysicalRegister check that I mentioned earlier.

[JesperAntonsson]

lib/CodeGen/MachineVerifier.cpp

993 ↗

(On Diff #103184)

Thanks for reviewing. Ok, so it's allowed to mix physregs and vregs? I'll upload a new patch that removes this check then.

test/CodeGen/MIR/X86/subregister-index-operands.mir

27 ↗

(On Diff #103184)

Agreed, it won't fail, so I'll just leave this test as is.

[JesperAntonsson]

Removed the check that tied physregs and virtregs aren't mixed. Removed changes in a test that had such mixes.

[JesperAntonsson]

Ping. How about it? Could we land this patch?

[qcolombet]

Could you add a .mir test case that exposes this verifier diagnostic? (In test/CodeGen/MIR then pick a target)

Other than that, LGTM.

[JesperAntonsson]

Added a MIR test for the check. Thanks to Quentin for input.

llvm/trunk/lib/CodeGen/MachineVerifier.cpp

if (MONum < NumDefs) {
     int TiedTo = MCID.getOperandConstraint(MONum, MCOI::TIED_TO);
     if (TiedTo != -1) {
       if (!MO->isReg())
         report("Tied use must be a register", MO, MONum);
       else if (!MO->isTied())
         report("Operand should be tied", MO, MONum);
       else if (unsigned(TiedTo) != MI->findTiedOperandIdx(MONum))
         report("Tied def doesn't match MCInstrDesc", MO, MONum);
+      else if (TargetRegisterInfo::isPhysicalRegister(MO->getReg())) {
+        const MachineOperand &MOTied = MI->getOperand(TiedTo);
+        if (!MOTied.isReg())
+          report("Tied counterpart must be a register", &MOTied, TiedTo);
+        else if (TargetRegisterInfo::isPhysicalRegister(MOTied.getReg()) &&
+                 MO->getReg() != MOTied.getReg())
+          report("Tied physical registers must match.", &MOTied, TiedTo);
+      }
     } else if (MO->isReg() && MO->isTied())
       report("Explicit operand should not be tied", MO, MONum);
   } else {
     // ARM adds %reg0 operands to indicate predicates. We'll allow that.
     if (MO->isReg() && !MO->isImplicit() && !MI->isVariadic() && MO->getReg())
       report("Extra explicit operand on non-variadic instruction", MO, MONum);
   }
 

llvm/trunk/test/CodeGen/MIR/X86/tied-physical-regs-match.mir

+# RUN: not llc -march=x86-64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# This test ensures that the Machine Verifier detects tied physical registers
+# that doesn't match.
+
+--- |
+
+  define i32 @foo() {
+  entry:
+    ret i32 0
+  }
+
+...
+---
+name:            foo
+body: |
+  bb.0.entry:
+    liveins: %rdi
+
+    ; CHECK: Tied physical registers must match.
+    %rbx = AND64rm killed %rdx, killed %rdi, 1, _, 0, _, implicit-def dead %eflags
+    RETQ %rbx
+...