[llvm] r324442 - GlobalISel: Always check operand types when executing match table

[Volkan Keles via llvm-commits]

Author: volkan
Date: Tue Feb  6 18:44:51 2018
New Revision: 324442

URL: http://llvm.org/viewvc/llvm-project?rev=324442&view=rev
Log:
GlobalISel: Always check operand types when executing match table

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.

Reviewers: dsanders, qcolombet

Reviewed By: qcolombet

Subscribers: qcolombet, rovka, kristof.beyls, llvm-commits

Differential Revision: https://reviews.llvm.org/D42984

Added:
    llvm/trunk/test/CodeGen/X86/GlobalISel/avoid-matchtable-crash.mir
Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h?rev=324442&r1=324441&r2=324442&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h Tue Feb  6 18:44:51 2018
@@ -293,8 +293,10 @@ bool InstructionSelector::executeMatchTa
                              << "]->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;
       }
@@ -319,11 +321,15 @@ bool InstructionSelector::executeMatchTa
 
       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) {
-        if (handleReject() == RejectAndGiveUp)
-          return false;
-      }
+      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: {
@@ -335,9 +341,10 @@ bool InstructionSelector::executeMatchTa
                              << 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;
       }
@@ -378,15 +385,19 @@ bool InstructionSelector::executeMatchTa
                              << "), Value=" << Value << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
 
-      // isOperandImmEqual() will sign-extend to 64-bits, so should we.
-      LLT Ty = MRI.getType(State.MIs[InsnID]->getOperand(OpIdx).getReg());
-      Value = SignExtend64(Value, Ty.getSizeInBits());
+      MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
+      if (MO.isReg()) {
+        // isOperandImmEqual() will sign-extend to 64-bits, so should we.
+        LLT Ty = MRI.getType(MO.getReg());
+        Value = SignExtend64(Value, Ty.getSizeInBits());
+
+        if (!isOperandImmEqual(MO, Value, MRI)) {
+          if (handleReject() == RejectAndGiveUp)
+            return false;
+        }
+      } else if (handleReject() == RejectAndGiveUp)
+        return false;
 
-      if (!isOperandImmEqual(State.MIs[InsnID]->getOperand(OpIdx), Value,
-                             MRI)) {
-        if (handleReject() == RejectAndGiveUp)
-          return false;
-      }
       break;
     }
 

Added: llvm/trunk/test/CodeGen/X86/GlobalISel/avoid-matchtable-crash.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/GlobalISel/avoid-matchtable-crash.mir?rev=324442&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/GlobalISel/avoid-matchtable-crash.mir (added)
+++ llvm/trunk/test/CodeGen/X86/GlobalISel/avoid-matchtable-crash.mir Tue Feb  6 18:44:51 2018
@@ -0,0 +1,36 @@
+# 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
+
+...