GlobalISel: Always check operand types when executing match table

[volkan]

Summary

Some of the commands tries to get the register without checking
if the specified operands is a register and causing crash. All commands
should check the type of the operand first and reject if the type is
not expected.

[qcolombet]

LGTM

llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

case GIM_CheckType: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t TypeID = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIM_CheckType(MIs[" << InsnID
                              << "]->getOperand(" << OpIdx
                              << "), TypeID=" << TypeID << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
-      if (MRI.getType(State.MIs[InsnID]->getOperand(OpIdx).getReg()) !=
-          ISelInfo.TypeObjects[TypeID]) {
+
+      MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
+      if (!MO.isReg() ||
+          MRI.getType(MO.getReg()) != ISelInfo.TypeObjects[TypeID]) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       }
       break;
     }
     case GIM_CheckPointerToAny: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t SizeInBits = MatchTable[CurrentIdx++];
 
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIM_CheckPointerToAny(MIs["
                              << InsnID << "]->getOperand(" << OpIdx
                              << "), SizeInBits=" << SizeInBits << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
 
       // iPTR must be looked up in the target.
       if (SizeInBits == 0) {
         MachineFunction *MF = State.MIs[InsnID]->getParent()->getParent();
         SizeInBits = MF->getDataLayout().getPointerSizeInBits(0);
       }
 
       assert(SizeInBits != 0 && "Pointer size must be known");
 
-      const LLT &Ty = MRI.getType(State.MIs[InsnID]->getOperand(OpIdx).getReg());
-      if (!Ty.isPointer() || Ty.getSizeInBits() != SizeInBits) {
+      MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
+      if (MO.isReg()) {
+        const LLT &Ty = MRI.getType(MO.getReg());
+        if (!Ty.isPointer() || Ty.getSizeInBits() != SizeInBits)
           if (handleReject() == RejectAndGiveUp)
             return false;
-      }
+      } else if (handleReject() == RejectAndGiveUp)
+        return false;
+
       break;
     }
     case GIM_CheckRegBankForClass: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t RCEnum = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIM_CheckRegBankForClass(MIs["
                              << InsnID << "]->getOperand(" << OpIdx
                              << "), RCEnum=" << RCEnum << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
-      if (&RBI.getRegBankFromRegClass(*TRI.getRegClass(RCEnum)) !=
-          RBI.getRegBank(State.MIs[InsnID]->getOperand(OpIdx).getReg(), MRI,
-                         TRI)) {
+      MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
+      if (!MO.isReg() ||
+          &RBI.getRegBankFromRegClass(*TRI.getRegClass(RCEnum)) !=
+              RBI.getRegBank(MO.getReg(), MRI, TRI)) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       }
       break;
     }
 
     case GIM_CheckComplexPattern: {
       int64_t InsnID = MatchTable[CurrentIdx++];
case GIM_CheckConstantInt: {
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t Value = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIM_CheckConstantInt(MIs["
                              << InsnID << "]->getOperand(" << OpIdx
                              << "), Value=" << Value << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
 
+      MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
+      if (MO.isReg()) {
         // isOperandImmEqual() will sign-extend to 64-bits, so should we.
-      LLT Ty = MRI.getType(State.MIs[InsnID]->getOperand(OpIdx).getReg());
+        LLT Ty = MRI.getType(MO.getReg());
         Value = SignExtend64(Value, Ty.getSizeInBits());
 
-      if (!isOperandImmEqual(State.MIs[InsnID]->getOperand(OpIdx), Value,
-                             MRI)) {
+        if (!isOperandImmEqual(MO, Value, MRI)) {
           if (handleReject() == RejectAndGiveUp)
             return false;
         }
+      } else if (handleReject() == RejectAndGiveUp)
+        return false;
+
       break;
     }
 
     case GIM_CheckLiteralInt: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t Value = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),

llvm/trunk/test/CodeGen/X86/GlobalISel/avoid-matchtable-crash.mir

+# RUN: not llc -o - -run-pass=instruction-select -pass-remarks-missed=gisel %s 2>&1 | FileCheck %s
+--- |
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64--linux-gnu"
+
+  define void @test_check_type() {
+    ret void
+  }
+...
+---
+name:            test_check_type
+alignment:       4
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+body:             |
+  bb.1 (%ir-block.0):
+    liveins: $edi
+
+    ; Intrinsic::x86_flags_read_u64 has a higher prority than
+    ; Intrinsic::x86_int in the match table and both of them
+    ; have two operands, but their IntrinsicID index is different.
+    ; This causes crash when executing GIM_CheckType for Intrinsic::x86_int
+    ; because Operand0 is not a register.
+    ; Make sure we check whether the first operand is a register and
+    ; reject if it's not.
+    ; CHECK: cannot select: G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.x86.int)
+    %1:gpr(s32) = COPY $edi
+    %0:gpr(s8) = G_TRUNC %1(s32)
+    G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.x86.int), %0(s8)
+    RET 0
+
+...