[llvm] 4fd8f11 - [MachineVerifier] Improve checks of target instructions operands.

[Jonas Paulsson via llvm-commits]

Author: Jonas Paulsson
Date: 2019-12-03T10:20:52+01:00
New Revision: 4fd8f11901b5bfb13a5fef597626dde31835873b

URL: https://github.com/llvm/llvm-project/commit/4fd8f11901b5bfb13a5fef597626dde31835873b
DIFF: https://github.com/llvm/llvm-project/commit/4fd8f11901b5bfb13a5fef597626dde31835873b.diff

LOG: [MachineVerifier]  Improve checks of target instructions operands.

While working with a patch for instruction selection, the splitting of a
large immediate ended up begin treated incorrectly by the backend. Where a
register operand should have been created, it instead became an immediate. To
my surprise the machine verifier failed to report this, which at the time
would have been helpful.

This patch improves the verifier so that it will report this type of error.

This patch XFAILs CodeGen/SPARC/fp128.ll, which has been reported at
https://bugs.llvm.org/show_bug.cgi?id=44091

Review: thegameg, arsenm, fhahn
https://reviews.llvm.org/D63973

Added: 
    llvm/test/MachineVerifier/verify-regops.mir

Modified: 
    llvm/lib/CodeGen/MachineVerifier.cpp
    llvm/test/CodeGen/SPARC/fp128.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 6d88aae70af3..517f2a941ba2 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1610,13 +1610,23 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
   } else if (MONum < MCID.getNumOperands()) {
     const MCOperandInfo &MCOI = MCID.OpInfo[MONum];
     // Don't check if it's the last operand in a variadic instruction. See,
-    // e.g., LDM_RET in the arm back end.
-    if (MO->isReg() &&
-        !(MI->isVariadic() && MONum == MCID.getNumOperands()-1)) {
-      if (MO->isDef() && !MCOI.isOptionalDef())
-        report("Explicit operand marked as def", MO, MONum);
-      if (MO->isImplicit())
-        report("Explicit operand marked as implicit", MO, MONum);
+    // e.g., LDM_RET in the arm back end. Check non-variadic operands only.
+    bool IsOptional = MI->isVariadic() && MONum == MCID.getNumOperands() - 1;
+    if (!IsOptional) {
+      if (MO->isReg()) {
+        if (MO->isDef() && !MCOI.isOptionalDef())
+          report("Explicit operand marked as def", MO, MONum);
+        if (MO->isImplicit())
+          report("Explicit operand marked as implicit", MO, MONum);
+      }
+
+      // Check that an instruction has register operands only as expected.
+      if (MCOI.OperandType == MCOI::OPERAND_REGISTER &&
+          !MO->isReg() && !MO->isFI())
+        report("Expected a register operand.", MO, MONum);
+      if ((MCOI.OperandType == MCOI::OPERAND_IMMEDIATE ||
+           MCOI.OperandType == MCOI::OPERAND_PCREL) && MO->isReg())
+        report("Expected a non-register operand.", MO, MONum);
     }
 
     int TiedTo = MCID.getOperandConstraint(MONum, MCOI::TIED_TO);

diff  --git a/llvm/test/CodeGen/SPARC/fp128.ll b/llvm/test/CodeGen/SPARC/fp128.ll
index 83912e0f211e..883e4718790b 100644
--- a/llvm/test/CodeGen/SPARC/fp128.ll
+++ b/llvm/test/CodeGen/SPARC/fp128.ll
@@ -3,6 +3,14 @@
 ; RUN: llc < %s -march=sparc -mattr=-hard-quad-float | FileCheck %s --check-prefix=CHECK --check-prefix=SOFT --check-prefix=BE
 ; RUN: llc < %s -march=sparcel -mattr=-hard-quad-float | FileCheck %s --check-prefix=CHECK --check-prefix=SOFT --check-prefix=EL
 
+; XFAIL: *
+; This test currently fails with expensive checks enabled, for more details see
+; https://bugs.llvm.org/show_bug.cgi?id=44091.
+; *** Bad machine code: Expected a register operand. ***
+; - function:    f128_compare
+; - basic block: %bb.0 entry (0x63f4028)
+; - instruction: CMPrr killed %21:intregs, 0, implicit-def $icc
+; - operand 1:   0
 
 ; CHECK-LABEL: f128_ops:
 ; CHECK:      ldd

diff  --git a/llvm/test/MachineVerifier/verify-regops.mir b/llvm/test/MachineVerifier/verify-regops.mir
new file mode 100644
index 000000000000..9219586ffc03
--- /dev/null
+++ b/llvm/test/MachineVerifier/verify-regops.mir
@@ -0,0 +1,37 @@
+# RUN: not llc -march=x86 -o - %s -run-pass=none -verify-machineinstrs \
+# RUN:   2>&1 | FileCheck %s
+# REQUIRES: x86-registered-target
+#
+# Check that MachineVerifier catches corrupt operands where MO->isReg()
+# returns true, but the descriptor says it should be an OPERAND_IMMEDIATE or
+# OPERAND_PCREL. Conversely, if MO->isReg() (and MO->isFI()) returns false,
+# check that not an OPERAND_REGISTER is expected.
+
+# CHECK-LABEL: fun
+
+# CHECK: *** Bad machine code: Expected a register operand. ***
+# CHECK: - instruction: %1:gr32 = XOR32rm -1, %fixed-stack.1, 1, $noreg, 0, $noreg, implicit-def dead $eflags :: (load 4 from %fixed-stack.1, align 8)
+# CHECK: - operand 1:   -1
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: %2:gr32 = OR32ri %1:gr32(tied-def 0), %0:gr32, implicit-def dead $eflags
+# CHECK: - operand 2:   %0:gr32
+
+
+name:            fun
+tracksRegLiveness: true
+fixedStack:
+  - { id: 1, offset: 8, size: 4, alignment: 8, isImmutable: true }
+  - { id: 3, size: 4, alignment: 16, isImmutable: true }
+body:             |
+  bb.0:
+    %0:gr32 = MOV32rm %fixed-stack.3, 1, $noreg, 0, $noreg :: (load 4 from %fixed-stack.3, align 16)
+    ; Was: %1:gr32 = XOR32rm %0, %fixed-stack.1, 1, $noreg, 0, $noreg, implicit-def dead $eflags :: (load 4 from %fixed-stack.1, align 8)
+    %1:gr32 = XOR32rm -1, %fixed-stack.1, 1, $noreg, 0, $noreg, implicit-def dead $eflags :: (load 4 from %fixed-stack.1, align 8)
+    ; Was: %2:gr32 = OR32ri %1, -256, implicit-def dead $eflags
+    %2:gr32 = OR32ri %1, %0, implicit-def dead $eflags
+    %3:gr32 = MOV32ri -1
+    $eax = COPY %2
+    $edx = COPY %3
+    RET 0, $eax, $edx
+...