TableGen/GlobalISel: Fix pattern matching of immarg literals

[arsenm]

Summary

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.

[paquette]

llvm/utils/TableGen/CodeGenInstruction.cpp

520–528

Can we reduce the nesting here a bit by returning false early?

[arsenm]

Early return and merge with isOperandAPointer

[paquette]

LGTM

[arsenm]

10edb1d0d4a15812a71f8953bba96a4f1fc9d0af

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

case GIM_CheckLiteralInt: {
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t Value = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIM_CheckLiteralInt(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.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;
+
+
       break;
     }
 
     case GIM_CheckIntrinsicID: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t Value = MatchTable[CurrentIdx++];
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),

llvm/test/TableGen/Common/GlobalISelEmitterCommon.td

 // Common target boilerplate for GlobalISelEmitter tests.
 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>;
 def F0 : Register<"f0"> { let Namespace = "MyTarget"; }
 def FPR32 : RegisterClass<"MyTarget", [f32], 32, (add F0)>;
 def FPR32Op : RegisterOperand<FPR32>;
 def p0 : PtrValueType <i32, 0>;
 
 class I<dag OOps, dag IOps, list<dag> Pat>
   : Instruction {
   let Namespace = "MyTarget";
   let OutOperandList = OOps;
   let InOperandList = IOps;
   let Pattern = Pat;
 }

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

+// 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.h

public:
     static std::string FlattenAsmStringVariants(StringRef AsmString,
                                                 unsigned Variant);
 
     // Is the specified operand in a generic instruction implicitly a pointer.
     // 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;
+
+    /// Check if the operand is required to be an immediate.
+    bool isOperandImmArg(unsigned i) const;
   };
 
 
   /// CodeGenInstAlias - This represents an InstAlias definition.
   class CodeGenInstAlias {
   public:
     Record *TheDef;            // The actual record defining this InstAlias.
 

llvm/utils/TableGen/CodeGenInstruction.cpp

if (i < ConstraintList->getNumArgs()) {
         return Constraint->getDef()->isSubClassOf("TypedOperand") &&
                Constraint->getDef()->getValueAsBit("IsPointer");
       }
     }
   }
   return false;
 }
 
+bool CodeGenInstruction::isOperandImmArg(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("IsImmediate");
+      }
+    }
+  }
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 /// CodeGenInstAlias Implementation
 //===----------------------------------------------------------------------===//
 
 /// tryAliasOpMatch - This is a helper function for the CodeGenInstAlias
 /// constructor.  It checks if an argument in an InstAlias pattern matches
 /// the corresponding operand of the instruction.  It returns true on a
 /// successful match, with ResOp set to the result operand to be used.

llvm/utils/TableGen/CodeGenIntrinsics.h

struct CodeGenIntrinsic {
 
   /// Returns true if the parameter at \p ParamIdx is a pointer type. Returns
   /// false if the parameter is not a pointer, or \p ParamIdx is greater than
   /// the size of \p IS.ParamVTs.
   ///
   /// Note that this requires that \p IS.ParamVTs is available.
   bool isParamAPointer(unsigned ParamIdx) const;
 
+  bool isParamImmArg(unsigned ParamIdx) const;
+
   CodeGenIntrinsic(Record *R);
 };
 
 class CodeGenIntrinsicTable {
   std::vector<CodeGenIntrinsic> Intrinsics;
 
 public:
   struct TargetSet {

llvm/utils/TableGen/CodeGenTarget.cpp

 }
 
 bool CodeGenIntrinsic::isParamAPointer(unsigned ParamIdx) const {
   if (ParamIdx >= IS.ParamVTs.size())
     return false;
   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

private:
   Expected<InstructionMatcher &>
   createAndImportSelDAGMatcher(RuleMatcher &Rule,
                                InstructionMatcher &InsnMatcher,
                                const TreePatternNode *Src, unsigned &TempOpIdx);
   Error importComplexPatternOperandMatcher(OperandMatcher &OM, Record *R,
                                            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,
       const TreePatternNode *Src, const TreePatternNode *Dst);
   Expected<action_iterator> createAndImportSubInstructionRenderer(
       action_iterator InsertPt, RuleMatcher &M, const TreePatternNode *Dst,
       unsigned TempReg);
   Expected<action_iterator>
bool IsIntrinsic =
         SrcGIOrNull->TheDef->getName() == "G_INTRINSIC_W_SIDE_EFFECTS";
     const CodeGenIntrinsic *II = Src->getIntrinsicInfo(CGP);
     if (IsIntrinsic && !II)
       return failedImport("Expected IntInit containing intrinsic ID)");
 
     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) {
         // For G_INTRINSIC/G_INTRINSIC_W_SIDE_EFFECTS, the operand immediately
         // following the defs is an intrinsic ID.
         if (i == 0) {
           OperandMatcher &OM =
               InsnMatcher.addOperand(OpIdx++, SrcChild->getName(), TempOpIdx);
           OM.addPredicate<IntrinsicIDOperandMatcher>(II);
           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);
     }
   }
 
   return InsnMatcher;
 }
 
 Error GlobalISelEmitter::importComplexPatternOperandMatcher(
if (auto *ChildDefInit = dyn_cast<DefInit>(SrcChild->getLeafValue())) {
         PhysReg = ChildRec;
       }
     }
   }
 
   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);
 
   OperandMatcher &OM = PhysReg ?
     InsnMatcher.addPhysRegInput(PhysReg, OpIdx, TempOpIdx) :
     InsnMatcher.addOperand(OpIdx, SrcChildName, TempOpIdx);
   if (OM.isSameAsAnotherOperand())
if (SrcChild->getOperator()->isSubClassOf("SDNode")) {
       }
       if (SrcChild->getOperator()->getName() == "timm") {
         OM.addPredicate<ImmOperandMatcher>();
         return Error::success();
       }
     }
   }
 
+  // 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()) {
     if (SrcChild->getOperator()->isSubClassOf("ComplexPattern")) {
       // When a ComplexPattern is used as an operator, it should do the same
       // thing as when used as a leaf. However, the children of the operator
       // name the sub-operands that make up the complex operand and we must
       // prepare to reference them in the renderer too.
if (!SrcChild->isLeaf()) {
     return Error::success();
   }
 
   if (SrcChild->hasAnyPredicate())
     return failedImport("Src pattern child has unsupported predicate");
 
   // Check for constant immediates.
   if (auto *ChildInt = dyn_cast<IntInit>(SrcChild->getLeafValue())) {
+    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();
   }
 
   // Check for def's like register classes or ComplexPattern's.
   if (auto *ChildDefInit = dyn_cast<DefInit>(SrcChild->getLeafValue())) {
     auto *ChildRec = ChildDefInit->getDef();
 
     // Check for register classes.