TableGen/GlobalISel: Fix pattern matching of immarg literals

For arguments that are not expected to be materialized with
G_CONSTANT, this was emitting predicates which could never match. It
was first adding a meaningless LLT check, which would always fail due
to the operand not being a register.

Infer the cases where a literal should check for an immediate operand,
instead of a register This avoids needing to invent a special way of
representing timm literal values.

Also handle immediate arguments in GIM_CheckLiteralInt. The comments
stated it handled isImm() and isCImm(), but that wasn't really true.

This unblocks work on the selection of all of the complicated AMDGPU
intrinsics in future commits.

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

@@ -642,10 +642,15 @@ bool InstructionSelector::executeMatchTable(
                              << "), Value=" << Value << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
       MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
-      if (!MO.isCImm() || !MO.getCImm()->equalsInt(Value)) {
-        if (handleReject() == RejectAndGiveUp)
-          return false;
-      }
+      if (MO.isImm() && MO.getImm() == Value)
+        break;
+
+      if (MO.isCImm() && MO.getCImm()->equalsInt(Value))
+        break;
+
+      if (handleReject() == RejectAndGiveUp)
+        return false;
+
       break;
     }
 

llvm/test/TableGen/Common/GlobalISelEmitterCommon.td

@@ -2,6 +2,10 @@
 def MyTargetISA : InstrInfo;
 def MyTarget : Target { let InstructionSet = MyTargetISA; }
 
+class MyTargetGenericInstruction : GenericInstruction {
+  let Namespace = "MyTarget";
+}
+
 def R0 : Register<"r0"> { let Namespace = "MyTarget"; }
 def GPR32 : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
 def GPR32Op : RegisterOperand<GPR32>;

llvm/test/TableGen/GlobalISelEmitter-immarg-literal-pattern.td

@@ -0,0 +1,62 @@
+// RUN: llvm-tblgen -gen-global-isel -warn-on-skipped-patterns -optimize-match-table=false -I %p/../../include -I %p/Common %s -o - | FileCheck -check-prefix=GISEL %s
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+def int_mytarget_sleep : Intrinsic<[], [llvm_i32_ty], [ImmArg<0>]>;
+
+def G_TGT_CAT : MyTargetGenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type1:$src0, untyped_imm_0:$immfield);
+}
+
+def TgtCat : SDNode<"MyTgt::CAT", SDTIntBinOp>;
+def : GINodeEquiv<G_TGT_CAT, TgtCat>;
+
+
+def SLEEP0 : I<(outs), (ins), []>;
+def SLEEP1 : I<(outs), (ins), []>;
+def CAT0 : I<(outs GPR32:$dst), (ins GPR32:$src0), []>;
+def CAT1 : I<(outs GPR32:$dst), (ins GPR32:$src0), []>;
+
+// Test immarg intrinsic pattern
+
+// Make sure there is no type check.
+// GISEL: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS,
+// GISEL: GIM_CheckIntrinsicID, /*MI*/0, /*Op*/0, Intrinsic::mytarget_sleep,
+// GISEL-NEXT: // MIs[0] Operand 1
+// GISEL-NEXT: GIM_CheckLiteralInt, /*MI*/0, /*Op*/1, 0,
+def : Pat<
+  (int_mytarget_sleep 0),
+  (SLEEP0)
+>;
+
+// GISEL: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS,
+// GISEL: GIM_CheckIntrinsicID, /*MI*/0, /*Op*/0, Intrinsic::mytarget_sleep,
+// GISEL-NEXT: // MIs[0] Operand 1
+// GISEL-NEXT: GIM_CheckLiteralInt, /*MI*/0, /*Op*/1, 1,
+def : Pat<
+  (int_mytarget_sleep 1),
+  (SLEEP1)
+>;
+
+// Check a non-intrinsic instruction with an immediate parameter.
+
+// GISEL: GIM_CheckOpcode, /*MI*/0, MyTarget::G_TGT_CAT,
+// GISEL: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
+// GISEL-NEXT: // MIs[0] Operand 2
+// GISEL-NEXT: GIM_CheckLiteralInt, /*MI*/0, /*Op*/2, 0,
+def : Pat<
+  (TgtCat i32:$src0, 0),
+  (CAT0 GPR32:$src0)
+>;
+
+// GISEL: GIM_CheckOpcode, /*MI*/0, MyTarget::G_TGT_CAT,
+// GISEL: GIM_CheckType, /*MI*/0, /*Op*/1, /*Type*/GILLT_s32,
+// GISEL-NEXT: // MIs[0] Operand 2
+// GISEL-NEXT: GIM_CheckLiteralInt, /*MI*/0, /*Op*/2, 93,
+def : Pat<
+  (TgtCat i32:$src0, 93),
+  (CAT1 GPR32:$src0)
+>;
+

llvm/utils/TableGen/CodeGenInstruction.cpp

@@ -504,16 +504,18 @@ FlattenAsmStringVariants(StringRef Cur, unsigned Variant) {
   return Res;
 }
 
-bool CodeGenInstruction::isOperandAPointer(unsigned i) const {
-  if (DagInit *ConstraintList = TheDef->getValueAsDag("InOperandList")) {
-    if (i < ConstraintList->getNumArgs()) {
-      if (DefInit *Constraint = dyn_cast<DefInit>(ConstraintList->getArg(i))) {
-        return Constraint->getDef()->isSubClassOf("TypedOperand") &&
-               Constraint->getDef()->getValueAsBit("IsPointer");
-      }
-    }
-  }
-  return false;
+bool CodeGenInstruction::isOperandImpl(unsigned i,
+                                       StringRef PropertyName) const {
+  DagInit *ConstraintList = TheDef->getValueAsDag("InOperandList");
+  if (!ConstraintList || i >= ConstraintList->getNumArgs())
+    return false;
+
+  DefInit *Constraint = dyn_cast<DefInit>(ConstraintList->getArg(i));
+  if (!Constraint)
+    return false;
+
+  return Constraint->getDef()->isSubClassOf("TypedOperand") &&
+         Constraint->getDef()->getValueAsBit(PropertyName);
 }
 
 //===----------------------------------------------------------------------===//

llvm/utils/TableGen/CodeGenInstruction.h

@@ -309,7 +309,17 @@ template <typename T> class ArrayRef;
     // This can be used on intructions that use typeN or ptypeN to identify
     // operands that should be considered as pointers even though SelectionDAG
     // didn't make a distinction between integer and pointers.
-    bool isOperandAPointer(unsigned i) const;
+    bool isOperandAPointer(unsigned i) const {
+      return isOperandImpl(i, "IsPointer");
+    }
+
+    /// Check if the operand is required to be an immediate.
+    bool isOperandImmArg(unsigned i) const {
+      return isOperandImpl(i, "IsImmediate");
+    }
+
+  private:
+    bool isOperandImpl(unsigned i, StringRef PropertyName) const;
   };
 
 

llvm/utils/TableGen/CodeGenIntrinsics.h

@@ -162,6 +162,8 @@ struct CodeGenIntrinsic {
   /// Note that this requires that \p IS.ParamVTs is available.
   bool isParamAPointer(unsigned ParamIdx) const;
 
+  bool isParamImmArg(unsigned ParamIdx) const;
+
   CodeGenIntrinsic(Record *R);
 };
 

llvm/utils/TableGen/CodeGenTarget.cpp

@@ -817,3 +817,9 @@ bool CodeGenIntrinsic::isParamAPointer(unsigned ParamIdx) const {
   MVT ParamType = MVT(IS.ParamVTs[ParamIdx]);
   return ParamType == MVT::iPTR || ParamType == MVT::iPTRAny;
 }
+
+bool CodeGenIntrinsic::isParamImmArg(unsigned ParamIdx) const {
+  std::pair<unsigned, ArgAttribute> Val = {ParamIdx, ImmArg};
+  return std::binary_search(ArgumentAttributes.begin(),
+                            ArgumentAttributes.end(), Val);
+}

llvm/utils/TableGen/GlobalISelEmitter.cpp

@@ -3314,8 +3314,8 @@ private:
                                            unsigned &TempOpIdx) const;
   Error importChildMatcher(RuleMatcher &Rule, InstructionMatcher &InsnMatcher,
                            const TreePatternNode *SrcChild,
-                           bool OperandIsAPointer, unsigned OpIdx,
-                           unsigned &TempOpIdx);
+                           bool OperandIsAPointer, bool OperandIsImmArg,
+                           unsigned OpIdx, unsigned &TempOpIdx);
 
   Expected<BuildMIAction &> createAndImportInstructionRenderer(
       RuleMatcher &M, InstructionMatcher &InsnMatcher,
@@ -3755,9 +3755,19 @@ Expected<InstructionMatcher &> GlobalISelEmitter::createAndImportSelDAGMatcher(
     for (unsigned i = 0; i != NumChildren; ++i) {
       TreePatternNode *SrcChild = Src->getChild(i);
 
+      // We need to determine the meaning of a literal integer based on the
+      // context. If this is a field required to be an immediate (such as an
+      // immarg intrinsic argument), the required predicates are different than
+      // a constant which may be materialized in a register. If we have an
+      // argument that is required to be an immediate, we should not emit an LLT
+      // type check, and should not be looking for a G_CONSTANT defined
+      // register.
+      bool OperandIsImmArg = SrcGIOrNull->isOperandImmArg(i);
+
       // SelectionDAG allows pointers to be represented with iN since it doesn't
       // distinguish between pointers and integers but they are different types in GlobalISel.
       // Coerce integers to pointers to address space 0 if the context indicates a pointer.
+      //
       bool OperandIsAPointer = SrcGIOrNull->isOperandAPointer(i);
 
       if (IsIntrinsic) {
@@ -3770,16 +3780,17 @@ Expected<InstructionMatcher &> GlobalISelEmitter::createAndImportSelDAGMatcher(
           continue;
         }
 
-        // We have to check intrinsics for llvm_anyptr_ty parameters.
+        // We have to check intrinsics for llvm_anyptr_ty and immarg parameters.
         //
         // Note that we have to look at the i-1th parameter, because we don't
         // have the intrinsic ID in the intrinsic's parameter list.
         OperandIsAPointer |= II->isParamAPointer(i - 1);
+        OperandIsImmArg |= II->isParamImmArg(i - 1);
       }
 
       if (auto Error =
               importChildMatcher(Rule, InsnMatcher, SrcChild, OperandIsAPointer,
-                                 OpIdx++, TempOpIdx))
+                                 OperandIsImmArg, OpIdx++, TempOpIdx))
         return std::move(Error);
     }
   }
@@ -3817,12 +3828,10 @@ static StringRef getSrcChildName(const TreePatternNode *SrcChild,
   return SrcChildName;
 }
 
-Error GlobalISelEmitter::importChildMatcher(RuleMatcher &Rule,
-                                            InstructionMatcher &InsnMatcher,
-                                            const TreePatternNode *SrcChild,
-                                            bool OperandIsAPointer,
-                                            unsigned OpIdx,
-                                            unsigned &TempOpIdx) {
+Error GlobalISelEmitter::importChildMatcher(
+    RuleMatcher &Rule, InstructionMatcher &InsnMatcher,
+    const TreePatternNode *SrcChild, bool OperandIsAPointer,
+    bool OperandIsImmArg, unsigned OpIdx, unsigned &TempOpIdx) {
 
   Record *PhysReg = nullptr;
   StringRef SrcChildName = getSrcChildName(SrcChild, PhysReg);
@@ -3852,10 +3861,14 @@ Error GlobalISelEmitter::importChildMatcher(RuleMatcher &Rule,
     }
   }
 
-  if (auto Error =
-          OM.addTypeCheckPredicate(ChildTypes.front(), OperandIsAPointer))
-    return failedImport(toString(std::move(Error)) + " for Src operand (" +
-                        to_string(*SrcChild) + ")");
+  // Immediate arguments have no meaningful type to check as they don't have
+  // registers.
+  if (!OperandIsImmArg) {
+    if (auto Error =
+            OM.addTypeCheckPredicate(ChildTypes.front(), OperandIsAPointer))
+      return failedImport(toString(std::move(Error)) + " for Src operand (" +
+                          to_string(*SrcChild) + ")");
+  }
 
   // Check for nested instructions.
   if (!SrcChild->isLeaf()) {
@@ -3906,7 +3919,13 @@ Error GlobalISelEmitter::importChildMatcher(RuleMatcher &Rule,
 
   // Check for constant immediates.
   if (auto *ChildInt = dyn_cast<IntInit>(SrcChild->getLeafValue())) {
-    OM.addPredicate<ConstantIntOperandMatcher>(ChildInt->getValue());
+    if (OperandIsImmArg) {
+      // Checks for argument directly in operand list
+      OM.addPredicate<LiteralIntOperandMatcher>(ChildInt->getValue());
+    } else {
+      // Checks for materialized constant
+      OM.addPredicate<ConstantIntOperandMatcher>(ChildInt->getValue());
+    }
     return Error::success();
   }