[MachineVerifier][GlobalISel] NFC, Improving MO printing and refactoring visitMachineInstrBefore

This is an NFC pre-commit for the following "Checking that generic
instrs have LLTs on all vregs" commit.

This overloads MachineOperand::print to make it possible to print LLTs
with standalone machine operands.

This also overloads MachineVerifier::print(...MachineOperand...) with
an optional LLT using the newly introduced MachineOperand::print
variant; no actual calls added.

This also refactors MachineVerifier::visitMachineInstrBefore in the
parts dealing with all generic instructions (checking Selected
property, LLTs, and phys regs).

llvm-svn: 331693

include/llvm/CodeGen/MachineOperand.h

@@ -295,6 +295,12 @@ public:
              unsigned TiedOperandIdx, const TargetRegisterInfo *TRI,
              const TargetIntrinsicInfo *IntrinsicInfo) const;
 
+  /// Same as print(os, TRI, IntrinsicInfo), but allows to specify the low-level
+  /// type to be printed the same way the full version of print(...) does it.
+  void print(raw_ostream &os, LLT TypeToPrint,
+             const TargetRegisterInfo *TRI = nullptr,
+             const TargetIntrinsicInfo *IntrinsicInfo = nullptr) const;
+
   void dump() const;
 
   //===--------------------------------------------------------------------===//

lib/CodeGen/MachineOperand.cpp

@@ -704,9 +704,15 @@ static void printCFI(raw_ostream &OS, const MCCFIInstruction &CFI,
 
 void MachineOperand::print(raw_ostream &OS, const TargetRegisterInfo *TRI,
                            const TargetIntrinsicInfo *IntrinsicInfo) const {
+  print(OS, LLT{}, TRI, IntrinsicInfo);
+}
+
+void MachineOperand::print(raw_ostream &OS, LLT TypeToPrint,
+                           const TargetRegisterInfo *TRI,
+                           const TargetIntrinsicInfo *IntrinsicInfo) const {
   tryToGetTargetInfo(*this, TRI, IntrinsicInfo);
   ModuleSlotTracker DummyMST(nullptr);
-  print(OS, DummyMST, LLT{}, /*PrintDef=*/false, /*IsStandalone=*/true,
+  print(OS, DummyMST, TypeToPrint, /*PrintDef=*/false, /*IsStandalone=*/true,
         /*ShouldPrintRegisterTies=*/true,
         /*TiedOperandIdx=*/0, TRI, IntrinsicInfo);
 }

lib/CodeGen/MachineVerifier.cpp

@@ -239,7 +239,8 @@ namespace {
     void report(const char *msg, const MachineFunction *MF);
     void report(const char *msg, const MachineBasicBlock *MBB);
     void report(const char *msg, const MachineInstr *MI);
-    void report(const char *msg, const MachineOperand *MO, unsigned MONum);
+    void report(const char *msg, const MachineOperand *MO, unsigned MONum,
+                LLT MOVRegType = LLT{});
 
     void report_context(const LiveInterval &LI) const;
     void report_context(const LiveRange &LR, unsigned VRegUnit,
@@ -490,15 +491,14 @@ void MachineVerifier::report(const char *msg, const MachineInstr *MI) {
   if (Indexes && Indexes->hasIndex(*MI))
     errs() << Indexes->getInstructionIndex(*MI) << '\t';
   MI->print(errs(), /*SkipOpers=*/true);
-  errs() << '\n';
 }
 
-void MachineVerifier::report(const char *msg,
-                             const MachineOperand *MO, unsigned MONum) {
+void MachineVerifier::report(const char *msg, const MachineOperand *MO,
+                             unsigned MONum, LLT MOVRegType) {
   assert(MO);
   report(msg, MO->getParent());
   errs() << "- operand " << MONum << ":   ";
-  MO->print(errs(), TRI);
+  MO->print(errs(), MOVRegType, TRI);
   errs() << "\n";
 }
 
@@ -877,11 +877,11 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
   if (MI->getNumOperands() < MCID.getNumOperands()) {
     report("Too few operands", MI);
     errs() << MCID.getNumOperands() << " operands expected, but "
-        << MI->getNumOperands() << " given.\n";
+           << MI->getNumOperands() << " given.\n";
   }
 
   if (MI->isPHI() && MF->getProperties().hasProperty(
-          MachineFunctionProperties::Property::NoPHIs))
+                         MachineFunctionProperties::Property::NoPHIs))
     report("Found PHI instruction with NoPHIs property set", MI);
 
   // Check the tied operands.
@@ -890,7 +890,8 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
 
   // Check the MachineMemOperands for basic consistency.
   for (MachineInstr::mmo_iterator I = MI->memoperands_begin(),
-       E = MI->memoperands_end(); I != E; ++I) {
+                                  E = MI->memoperands_end();
+       I != E; ++I) {
     if ((*I)->isLoad() && !MI->mayLoad())
       report("Missing mayLoad flag", MI);
     if ((*I)->isStore() && !MI->mayStore())
@@ -913,31 +914,31 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
     }
   }
 
-  // Check types.
   if (isPreISelGenericOpcode(MCID.getOpcode())) {
     if (isFunctionSelected)
       report("Unexpected generic instruction in a Selected function", MI);
 
-    // Generic instructions specify equality constraints between some
-    // of their operands. Make sure these are consistent.
+    // Check types.
     SmallVector<LLT, 4> Types;
-    for (unsigned i = 0; i < MCID.getNumOperands(); ++i) {
-      if (!MCID.OpInfo[i].isGenericType())
+    for (unsigned I = 0; I < MCID.getNumOperands(); ++I) {
+      if (!MCID.OpInfo[I].isGenericType())
         continue;
-      size_t TypeIdx = MCID.OpInfo[i].getGenericTypeIndex();
+      // Generic instructions specify type equality constraints between some of
+      // their operands. Make sure these are consistent.
+      size_t TypeIdx = MCID.OpInfo[I].getGenericTypeIndex();
       Types.resize(std::max(TypeIdx + 1, Types.size()));
 
-      LLT OpTy = MRI->getType(MI->getOperand(i).getReg());
+      const MachineOperand *MO = &MI->getOperand(I);
+      LLT OpTy = MRI->getType(MO->getReg());
       if (Types[TypeIdx].isValid() && Types[TypeIdx] != OpTy)
         report("type mismatch in generic instruction", MI);
       Types[TypeIdx] = OpTy;
     }
-  }
 
-  // Generic opcodes must not have physical register operands.
-  if (isPreISelGenericOpcode(MCID.getOpcode())) {
-    for (auto &Op : MI->operands()) {
-      if (Op.isReg() && TargetRegisterInfo::isPhysicalRegister(Op.getReg()))
+    // Generic opcodes must not have physical register operands.
+    for (unsigned I = 0; I < MI->getNumOperands(); ++I) {
+      const MachineOperand *MO = &MI->getOperand(I);
+      if (MO->isReg() && TargetRegisterInfo::isPhysicalRegister(MO->getReg()))
         report("Generic instruction cannot have physical register", MI);
     }
   }

test/CodeGen/X86/verifier-generic-types-1.mir

@@ -0,0 +1,15 @@
+# RUN: not llc -o - %s -mtriple=x86_64-- -verify-machineinstrs -run-pass=none 2>&1
+# XFAIL: *
+
+---
+name:              first_type_of_a_type_index_missing_and_no_mismatches
+tracksRegLiveness: true
+body:              |
+  bb.0:
+    liveins: $rdi, $rsi
+
+    %0:_(s64) = COPY $rdi
+    %1:gr64 = COPY $rsi
+    %2:gr64 = G_ADD %0, %1
+    $rax = COPY %2
+...

test/CodeGen/X86/verifier-generic-types-2.mir

@@ -0,0 +1,19 @@
+# RUN: not llc -o - %s -mtriple=x86_64-- -verify-machineinstrs -run-pass=none 2>&1 | FileCheck %s
+
+# CHECK: Bad machine code: type mismatch in generic instruction
+# CHECK-NEXT: - function:    first_type_of_a_type_index_missing_and_a_mismatch
+# CHECK-NEXT: - basic block: %bb.0
+# CHECK-NEXT: - instruction: %2:gr64 = G_ADD %0:_, %1:_
+
+---
+name:              first_type_of_a_type_index_missing_and_a_mismatch
+tracksRegLiveness: true
+body:              |
+  bb.0:
+    liveins: $rdi, $esi
+
+    %0:_(s64) = COPY $rdi
+    %1:_(s32)= COPY $esi
+    %2:gr64 = G_ADD %0, %1
+    $rax = COPY %2
+...