[SystemZ] Improve verification of MachineOperands.

[jonpa]

Summary

Now that the machine verifier will check for cases of register/immediate MachineOperands and their correspondence to the MC instruction descriptor, this patch adds the operand types to the descriptors where they were previously missing. All MCOI::OPERAND_UNKNOWN operand types have been handled to get a known type, except for G_... (global isel) instructions.

This involves:

• Immediate operands becoming MCOI::OPERAND_IMMEDIATE
• Addressing modes, which due to the complex Tablegen modelling already present now get MCOI::OPERAND_MEMORY as their type. All the addressing mode individual operands (registers and displacements) get this type, so their consistency with the actual built MachineOperand objects must be checked in SystemZInstrInfo::verifyInstruction(). It would have been nice to instead directly give the types MCOI::OPERAND_REGISTER and MCOI_OPERAND_IMMEDIATE, but I did not manage to do so.
• PC-relative operands get MCOI::OPERAND_PCREL.

(In the test, the MVCLoop instruction with a register instead of imm64 causes isImm() to trigger an assert on trunk, but the test checks that this is caught by the Machine verifier with the patch.)

[uweigand]

LGTM, thanks!

[jonpa]

49f55dd

llvm/lib/Target/SystemZ/SystemZInstrInfo.h

public:
   unsigned getLoadAndTrap(unsigned Opcode) const;
 
   // Emit code before MBBI in MI to move immediate value Value into
   // physical register Reg.
   void loadImmediate(MachineBasicBlock &MBB,
                      MachineBasicBlock::iterator MBBI,
                      unsigned Reg, uint64_t Value) const;
 
+  // Perform target specific instruction verification.
+  bool verifyInstruction(const MachineInstr &MI,
+                         StringRef &ErrInfo) const override;
+
   // Sometimes, it is possible for the target to tell, even without
   // aliasing information, that two MIs access different memory
   // addresses. This function returns true if two MIs access different
   // memory addresses and false otherwise.
   bool
   areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
                                   const MachineInstr &MIb) const override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZINSTRINFO_H

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

else if (SystemZ::isImmLH(Value)) {
     Value >>= 16;
   } else {
     assert(isInt<32>(Value) && "Huge values not handled yet");
     Opcode = SystemZ::LGFI;
   }
   BuildMI(MBB, MBBI, DL, get(Opcode), Reg).addImm(Value);
 }
 
+bool SystemZInstrInfo::verifyInstruction(const MachineInstr &MI,
+                                         StringRef &ErrInfo) const {
+  const MCInstrDesc &MCID = MI.getDesc();
+  for (unsigned I = 0, E = MI.getNumOperands(); I != E; ++I) {
+    if (I >= MCID.getNumOperands())
+      break;
+    const MachineOperand &Op = MI.getOperand(I);
+    const MCOperandInfo &MCOI = MCID.OpInfo[I];
+    // Addressing modes have register and immediate operands. Op should be a
+    // register (or frame index) operand if MCOI.RegClass contains a valid
+    // register class, or an immediate otherwise.
+    if (MCOI.OperandType == MCOI::OPERAND_MEMORY &&
+        ((MCOI.RegClass != -1 && !Op.isReg() && !Op.isFI()) ||
+         (MCOI.RegClass == -1 && !Op.isImm()))) {
+      ErrInfo = "Addressing mode operands corrupt!";
+      return false;
+    }
+  }
+
+  return true;
+}
+
 bool SystemZInstrInfo::
 areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
                                 const MachineInstr &MIb) const {
 
   if (!MIa.hasOneMemOperand() || !MIb.hasOneMemOperand())
     return false;
 
   // If mem-operands show that the same address Value is used by both

llvm/lib/Target/SystemZ/SystemZOperands.td

class ImmediateTLSAsmOperand<string name>
   let Name = name;
   let RenderMethod = "addImmTLSOperands";
 }
 
 class ImmediateOp<ValueType vt, string asmop> : Operand<vt> {
   let PrintMethod = "print"##asmop##"Operand";
   let DecoderMethod = "decode"##asmop##"Operand";
   let ParserMatchClass = !cast<AsmOperandClass>(asmop);
+  let OperandType = "OPERAND_IMMEDIATE";
 }
 
 class ImmOpWithPattern<ValueType vt, string asmop, code pred, SDNodeXForm xform,
       SDNode ImmNode = imm> :
   ImmediateOp<vt, asmop>, PatLeaf<(vt ImmNode), pred, xform>;
 
 // class ImmediatePatLeaf<ValueType vt, code pred,
 //       SDNodeXForm xform, SDNode ImmNode>

 class PCRelTLSAsmOperand<string size>
   : ImmediateTLSAsmOperand<"PCRelTLS"##size> {
   let PredicateMethod = "isImmTLS";
   let ParserMethod = "parsePCRelTLS"##size;
 }
 
 // Constructs an operand for a PC-relative address with address type VT.
 // ASMOP is the associated asm operand.
+let OperandType = "OPERAND_PCREL" in {
   class PCRelOperand<ValueType vt, AsmOperandClass asmop> : Operand<vt> {
     let PrintMethod = "printPCRelOperand";
     let ParserMatchClass = asmop;
   }
   class PCRelTLSOperand<ValueType vt, AsmOperandClass asmop> : Operand<vt> {
     let PrintMethod = "printPCRelTLSOperand";
     let ParserMatchClass = asmop;
   }
+}
 
 // Constructs both a DAG pattern and instruction operand for a PC-relative
 // address with address size VT.  SELF is the name of the operand and
 // ASMOP is the associated asm operand.
 class PCRelAddress<ValueType vt, string self, AsmOperandClass asmop>
   : ComplexPattern<vt, 1, "selectPCRelAddress",
                    [z_pcrel_wrapper, z_pcrel_offset]>,
     PCRelOperand<vt, asmop> {

 // (base register, displacement, etc.).
 class AddressOperand<string bitsize, string dispsize, string length,
                      string format, dag operands>
   : Operand<!cast<ValueType>("i"##bitsize)> {
   let PrintMethod = "print"##format##"Operand";
   let EncoderMethod = "get"##format##dispsize##length##"Encoding";
   let DecoderMethod =
     "decode"##format##bitsize##"Disp"##dispsize##length##"Operand";
+  let OperandType = "OPERAND_MEMORY";
   let MIOperandInfo = operands;
   let ParserMatchClass =
     !cast<AddressAsmOperand>(format##bitsize##"Disp"##dispsize##length);
 }
 
 // Constructs both a DAG pattern and instruction operand for an addressing mode.
 // FORMAT, BITSIZE, DISPSIZE and LENGTH are the parameters to an associated
 // AddressAsmOperand.  OPERANDS is a list of NUMOPS individual operands

 defm imm64zx32n : Immediate<i64, [{
   return isUInt<32>(-N->getSExtValue());
 }], NEGUIMM32, "U32Imm">;
 
 defm imm64zx48 : Immediate<i64, [{
   return isUInt<64>(N->getZExtValue());
 }], UIMM48, "U48Imm">;
 
+let OperandType = "OPERAND_IMMEDIATE" in
   def imm64 : ImmLeaf<i64, [{}]>, Operand<i64>;
 
 //===----------------------------------------------------------------------===//
 // Floating-point immediates
 //===----------------------------------------------------------------------===//
 
 // Floating-point zero.
 def fpimm0 : PatLeaf<(fpimm), [{ return N->isExactlyValue(+0.0); }]>;
 

 //===----------------------------------------------------------------------===//
 // Miscellaneous
 //===----------------------------------------------------------------------===//
 
 // A 4-bit condition-code mask.
 def cond4 : PatLeaf<(i32 timm), [{ return (N->getZExtValue() < 16); }]>,
             Operand<i32> {
   let PrintMethod = "printCond4Operand";
+  let OperandType = "OPERAND_IMMEDIATE";
 }

llvm/test/CodeGen/SystemZ/mverify-optypes.mir

+# RUN: not llc -mtriple=s390x-linux-gnu -mcpu=z14 -run-pass=none -o - %s \
+# RUN:  2>&1 | FileCheck %s
+# REQUIRES: asserts
+#
+# Test that the machine verifier catches wrong operand types.
+
+--- |
+  define anyregcc i64 @fun() { ret i64 0  }
+  @gsrc = global i16 2
+...
+---
+name:            fun
+alignment:       16
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gr64bit }
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0 (%ir-block.0):
+    liveins: $r2l
+    %0:gr64bit = LGHI $r2l
+    %1:addr64bit = LLILL $r2l
+    %2:addr64bit = LGFI $r2l
+    %3:addr64bit = LGFI 0
+
+    %3:addr64bit = AG %3, %1, 0, %2, implicit-def dead $cc
+    %3:addr64bit = AG %3, %1, %0, %2, implicit-def dead $cc
+    %3:addr64bit = AG %3, %1, 0, 2, implicit-def dead $cc
+
+    %4:addr64bit = LARL @gsrc
+    %4:addr64bit = LARL $r2l
+
+    MVCLoop %4, 0, %3, 0, 1680, %0, implicit-def $cc
+    MVCLoop %4, 0, %3, 0, %1, %0, implicit-def $cc
+
+    BCR 0, 0, $r2d, implicit $cc
+    BCR 0, $r2d, $r2d, implicit $cc
+    Return implicit $r2d
+
+...
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: %0:gr64bit = LGHI $r2l
+# CHECK: - operand 1:   $r2l
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: %1:addr64bit = LLILL $r2l
+# CHECK: - operand 1:   $r2l
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: %2:addr64bit = LGFI $r2l
+# CHECK: - operand 1:   $r2l
+
+# CHECK: *** Bad machine code: Addressing mode operands corrupt! ***
+# CHECK: - instruction: %3:addr64bit = AG %3:addr64bit(tied-def 0), %1:addr64bit, %0:gr64bit, %2:addr64bit, implicit-def dead $cc
+
+# CHECK: *** Bad machine code: Addressing mode operands corrupt! ***
+# CHECK: - instruction: %3:addr64bit = AG %3:addr64bit(tied-def 0), %1:addr64bit, 0, 2, implicit-def dead $cc
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: %4:addr64bit = LARL $r2l
+# CHECK: - operand 1:   $r2l
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: MVCLoop %4:addr64bit, 0, %3:addr64bit, 0, %1:addr64bit, %0:gr64bit, implicit-def $cc
+# CHECK: - operand 4:   %1:addr64bit
+
+# CHECK: *** Bad machine code: Expected a non-register operand. ***
+# CHECK: - instruction: BCR 0, $r2d, $r2d, implicit $cc
+# CHECK: - operand 1:   $r2d