[globalisel][tablegen] Add experimental support for OperandWithDefaultOps, PredicateOperand, and OptionalDefOperand

[dsanders]

Summary

As far as instruction selection is concerned, all three appear to be same thing.

Support for these operands is experimental since AArch64 doesn't make use
of them and the in-tree targets that do use them (AMDGPU for
OperandWithDefaultOps, AMDGPU/ARM/Hexagon/Lanai for PredicateOperand, and ARM
for OperandWithDefaultOps) are not using tablegen-erated GlobalISel yet.

[dsanders]

Add a default register test

[dsanders]

Add the test case without accidentally removing the rest of the patch.

[ab]

This seems to only add optional uses, what about OptionalDefOperand? (I'm fine with "we don't need that", but I don't see anything that would make the import fail)

[rovka]

test/TableGen/GlobalISelEmitter.td

29 ↗

(On Diff #92911)

Could you also add a test with 2 ops for the same operand? That would correspond to ARM's predicate operand.

242 ↗

(On Diff #92911)

Sorry I didn't notice this before, but I'm not sure this is a good use for CHECK-LABEL. IIUC the labels are supposed to be unique within the file. I might be wrong about it, but I thought I should bring it up just in case... Maybe we could generate some comment next to each lambda to disambiguate between them?

utils/TableGen/GlobalISelEmitter.cpp

36 ↗

(On Diff #92911)

Nit: This is not properly ordered (should go above Statistic.h)

1436 ↗

(On Diff #92911)

I'm a bit nervous about what happens if we *do* have a value for one of the default operands. Won't that mess up the whole algorithm? AFAICT we're not bailing out if we end up with too many operands...

1437 ↗

(On Diff #92911)

Nit: ARM has lots of instructions with 2 default operands (the pred and the condition code). I think it wouldn't hurt much to use 2 here to avoid going to the heap in those cases.

[dsanders]

In D31135#715631, @ab wrote:

This seems to only add optional uses, what about OptionalDefOperand? (I'm fine with "we don't need that", but I don't see anything that would make the import fail)

I think this patch implements OptionalDefOperand but the ISel aspects of OptionalDefOperand are a bit strange. There's three uses of it in LLVM at the moment, all of them in ARM:

• ldstm_mode is never used as far as I can tell and the default operand is '1' which doesn't make sense for an explicit def
• Despite its name, cc_out only ever appears at the end of the input operand list (see the sI class) and seems to behave like OperandWithDefaultOps
• s_cc_out is used as an optional def but only appears in InstAlias's

test/TableGen/GlobalISelEmitter.td

29 ↗

(On Diff #92911)

Ok

242 ↗

(On Diff #92911)

There's no requirement that they're unique, it's just that CHECK-LABEL can match in unexpected places if they're not distinct enough. In this case, it's being used to chop up the input into individual rules so that CHECK doesn't match in another rules code although this is moot now that we only have CHECK-NEXT's.

Maybe we could generate some comment next to each lambda to disambiguate between them?

Now that you mention it, the pattern comment (e.g. line 253) should probably be in front of the 'if ([&]() {' line. I think that's for a different patch though.

utils/TableGen/GlobalISelEmitter.cpp

36 ↗

(On Diff #92911)

Yep

1436 ↗

(On Diff #92911)

Are you thinking of the case where the OperandWithDefaultOps has 2 operands and a Pat<> provides just one? Yes, that would mess up the algorithm. I don't think anyone has used OperandWithDefaultOps like that but I should at least check for it so we fail nicely if someone has done that.

1437 ↗

(On Diff #92911)

Ok

[dsanders]

Add 2-default-operand test case
Check for too many operands
Nits

[rovka]

LGTM, thanks.

llvm/trunk/include/llvm/TableGen/Record.h

public:
 
   std::string getAsString() const override;
 
   typedef SmallVectorImpl<Init*>::const_iterator       const_arg_iterator;
   typedef SmallVectorImpl<StringInit*>::const_iterator const_name_iterator;
 
   inline const_arg_iterator  arg_begin() const { return Args.begin(); }
   inline const_arg_iterator  arg_end  () const { return Args.end();   }
+  inline iterator_range<const_arg_iterator> args() const {
+    return llvm::make_range(arg_begin(), arg_end());
+  }
 
   inline size_t              arg_size () const { return Args.size();  }
   inline bool                arg_empty() const { return Args.empty(); }
 
   inline const_name_iterator name_begin() const { return ArgNames.begin(); }
   inline const_name_iterator name_end  () const { return ArgNames.end();   }
 
   inline size_t              name_size () const { return ArgNames.size();  }

llvm/trunk/test/TableGen/GlobalISelEmitter.td

 
 def complex : Operand<i32>, ComplexPattern<i32, 2, "SelectComplexPattern", []> {
   let MIOperandInfo = (ops i32imm, i32imm);
 }
 def gi_complex :
     GIComplexOperandMatcher<s32, (ops i32imm, i32imm), "selectComplexPattern">,
     GIComplexPatternEquiv<complex>;
 
+def m1 : OperandWithDefaultOps <i32, (ops (i32 -1))>;
+def Z : OperandWithDefaultOps <i32, (ops R0)>;
+def m1Z : OperandWithDefaultOps <i32, (ops (i32 -1), R0)>;
+
 //===- Test the function definition boilerplate. --------------------------===//
 
 // CHECK: bool MyTargetInstructionSelector::selectImpl(MachineInstr &I) const {
 // CHECK: MachineFunction &MF = *I.getParent()->getParent();
 // CHECK: const MachineRegisterInfo &MRI = MF.getRegInfo();
 
 //===- Test a pattern with multiple ComplexPattern operands. --------------===//
 //

 // CHECK-NEXT:          MachineInstr &NewI = *MIB;
 // CHECK-NEXT:          constrainSelectedInstRegOperands(NewI, TII, TRI, RBI);
 // CHECK-NEXT:          return true;
 // CHECK-NEXT:        }
 
 def INSN1 : I<(outs GPR32:$dst), (ins GPR32:$src1, complex:$src2), []>;
 def : Pat<(sub GPR32:$src1, complex:$src2), (INSN1 GPR32:$src1, complex:$src2)>;
 
+//===- Test a simple pattern with a default operand. ----------------------===//
+//
+
+// CHECK-LABEL: if ([&]() {
+// CHECK-NEXT:    MachineInstr &MI0 = I;
+// CHECK-NEXT:    if (MI0.getNumOperands() < 3)
+// CHECK-NEXT:      return false;
+// CHECK-NEXT:    if ((MI0.getOpcode() == TargetOpcode::G_XOR) &&
+// CHECK-NEXT:        ((/* dst */ (MRI.getType(MI0.getOperand(0).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(0).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(1).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* Operand 2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:        (isOperandImmEqual(MI0.getOperand(2), -2, MRI))))) {
+// CHECK-NEXT:      // (xor:i32 GPR32:i32:$src1, -2:i32) => (XORI:i32 GPR32:i32:$src1)
+// CHECK-NEXT:      MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::XORI));
+// CHECK-NEXT:      MIB.add(MI0.getOperand(0)/*dst*/);
+// CHECK-NEXT:      MIB.addImm(-1);
+// CHECK-NEXT:      MIB.add(MI0.getOperand(1)/*src1*/);
+// CHECK-NEXT:      for (const auto *FromMI : {&MI0, })
+// CHECK-NEXT:        for (const auto &MMO : FromMI->memoperands())
+// CHECK-NEXT:          MIB.addMemOperand(MMO);
+// CHECK-NEXT:      I.eraseFromParent();
+// CHECK-NEXT:      MachineInstr &NewI = *MIB;
+// CHECK-NEXT:      constrainSelectedInstRegOperands(NewI, TII, TRI, RBI);
+// CHECK-NEXT:      return true;
+// CHECK-NEXT:    }
+// CHECK-NEXT:    return false;
+// CHECK-NEXT:  }()) { return true; }
+
+// The -2 is just to distinguish it from the 'not' case below.
+def XORI : I<(outs GPR32:$dst), (ins m1:$src2, GPR32:$src1),
+             [(set GPR32:$dst, (xor GPR32:$src1, -2))]>;
+
+//===- Test a simple pattern with a default register operand. -------------===//
+//
+
+// CHECK-LABEL: if ([&]() {
+// CHECK-NEXT:    MachineInstr &MI0 = I;
+// CHECK-NEXT:    if (MI0.getNumOperands() < 3)
+// CHECK-NEXT:      return false;
+// CHECK-NEXT:    if ((MI0.getOpcode() == TargetOpcode::G_XOR) &&
+// CHECK-NEXT:        ((/* dst */ (MRI.getType(MI0.getOperand(0).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(0).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(1).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* Operand 2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:        (isOperandImmEqual(MI0.getOperand(2), -3, MRI))))) {
+// CHECK-NEXT:      // (xor:i32 GPR32:i32:$src1, -3:i32) => (XOR:i32 GPR32:i32:$src1)
+// CHECK-NEXT:      MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::XOR));
+// CHECK-NEXT:      MIB.add(MI0.getOperand(0)/*dst*/);
+// CHECK-NEXT:      MIB.addReg(MyTarget::R0);
+// CHECK-NEXT:      MIB.add(MI0.getOperand(1)/*src1*/);
+// CHECK-NEXT:      for (const auto *FromMI : {&MI0, })
+// CHECK-NEXT:        for (const auto &MMO : FromMI->memoperands())
+// CHECK-NEXT:          MIB.addMemOperand(MMO);
+// CHECK-NEXT:      I.eraseFromParent();
+// CHECK-NEXT:      MachineInstr &NewI = *MIB;
+// CHECK-NEXT:      constrainSelectedInstRegOperands(NewI, TII, TRI, RBI);
+// CHECK-NEXT:      return true;
+// CHECK-NEXT:    }
+// CHECK-NEXT:    return false;
+// CHECK-NEXT:  }()) { return true; }
+
+// The -3 is just to distinguish it from the 'not' case below and the other default op case above.
+def XOR : I<(outs GPR32:$dst), (ins Z:$src2, GPR32:$src1),
+            [(set GPR32:$dst, (xor GPR32:$src1, -3))]>;
+
+//===- Test a simple pattern with a multiple default operands. ------------===//
+//
+
+// CHECK-LABEL: if ([&]() {
+// CHECK-NEXT:    MachineInstr &MI0 = I;
+// CHECK-NEXT:    if (MI0.getNumOperands() < 3)
+// CHECK-NEXT:      return false;
+// CHECK-NEXT:    if ((MI0.getOpcode() == TargetOpcode::G_XOR) &&
+// CHECK-NEXT:        ((/* dst */ (MRI.getType(MI0.getOperand(0).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(0).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(1).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* Operand 2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:        (isOperandImmEqual(MI0.getOperand(2), -4, MRI))))) {
+// CHECK-NEXT:      // (xor:i32 GPR32:i32:$src1, -4:i32) => (XORlike:i32 GPR32:i32:$src1)
+// CHECK-NEXT:      MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::XORlike));
+// CHECK-NEXT:      MIB.add(MI0.getOperand(0)/*dst*/);
+// CHECK-NEXT:      MIB.addImm(-1);
+// CHECK-NEXT:      MIB.addReg(MyTarget::R0);
+// CHECK-NEXT:      MIB.add(MI0.getOperand(1)/*src1*/);
+// CHECK-NEXT:      for (const auto *FromMI : {&MI0, })
+// CHECK-NEXT:        for (const auto &MMO : FromMI->memoperands())
+// CHECK-NEXT:          MIB.addMemOperand(MMO);
+// CHECK-NEXT:      I.eraseFromParent();
+// CHECK-NEXT:      MachineInstr &NewI = *MIB;
+// CHECK-NEXT:      constrainSelectedInstRegOperands(NewI, TII, TRI, RBI);
+// CHECK-NEXT:      return true;
+// CHECK-NEXT:    }
+// CHECK-NEXT:    return false;
+// CHECK-NEXT:  }()) { return true; }
+
+// The -4 is just to distinguish it from the other 'not' cases.
+def XORlike : I<(outs GPR32:$dst), (ins m1Z:$src2, GPR32:$src1),
+                [(set GPR32:$dst, (xor GPR32:$src1, -4))]>;
+
 //===- Test a simple pattern with constant immediate operands. ------------===//
 //
 // This must precede the 3-register variants because constant immediates have
 // priority over register banks.
 
 // CHECK-LABEL: if ([&]() {
 // CHECK-NEXT:    MachineInstr &MI0 = I;
 // CHECK-NEXT:    if (MI0.getNumOperands() < 3)

llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp

 ///
 /// FIXME: We'll probably want to eventually define a base
 /// "TargetGenInstructionSelector" class.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "CodeGenDAGPatterns.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineValueType.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/LowLevelTypeImpl.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
void OperandPlaceholder::emitCxxValueExpr(raw_ostream &OS) const {
   case OP_Temporary:
     OS << "TempOp" << Temporary.OpIdx;
     break;
   }
 }
 
 class OperandRenderer {
 public:
-  enum RendererKind { OR_Copy, OR_Register, OR_ComplexPattern };
+  enum RendererKind { OR_Copy, OR_Imm, OR_Register, OR_ComplexPattern };
 
 protected:
   RendererKind Kind;
 
 public:
   OperandRenderer(RendererKind Kind) : Kind(Kind) {}
   virtual ~OperandRenderer() {}
 
public:
   }
 
   void emitCxxRenderStmts(raw_ostream &OS, RuleMatcher &Rule) const override {
     OS << "    MIB.addReg(" << RegisterDef->getValueAsString("Namespace")
        << "::" << RegisterDef->getName() << ");\n";
   }
 };
 
+/// Adds a specific immediate to the instruction being built.
+class ImmRenderer : public OperandRenderer {
+protected:
+  int64_t Imm;
+
+public:
+  ImmRenderer(int64_t Imm)
+      : OperandRenderer(OR_Imm), Imm(Imm) {}
+
+  static bool classof(const OperandRenderer *R) {
+    return R->getKind() == OR_Imm;
+  }
+
+  void emitCxxRenderStmts(raw_ostream &OS, RuleMatcher &Rule) const override {
+    OS << "    MIB.addImm(" << Imm << ");\n";
+  }
+};
+
 class RenderComplexPatternOperand : public OperandRenderer {
 private:
   const Record &TheDef;
   std::vector<OperandPlaceholder> Sources;
 
   unsigned getNumOperands() const {
     return TheDef.getValueAsDag("Operands")->getNumArgs();
   }
Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
   auto &DstMIBuilder = M.addAction<BuildMIAction>(&DstI, InsnMatcher);
 
   // Render the explicit defs.
   for (unsigned I = 0; I < DstI.Operands.NumDefs; ++I) {
     const auto &DstIOperand = DstI.Operands[I];
     DstMIBuilder.addRenderer<CopyRenderer>(InsnMatcher, DstIOperand.Name);
   }
 
+  // Figure out which operands need defaults inserted. Operands that subclass
+  // OperandWithDefaultOps are considered from left to right until we have
+  // enough operands to render the instruction.
+  SmallSet<unsigned, 2> DefaultOperands;
+  unsigned DstINumUses = DstI.Operands.size() - DstI.Operands.NumDefs;
+  unsigned NumDefaultOperands = 0;
+  for (unsigned I = 0; I < DstINumUses &&
+                       DstINumUses > Dst->getNumChildren() + NumDefaultOperands;
+       ++I) {
+    const auto &DstIOperand = DstI.Operands[DstI.Operands.NumDefs + I];
+    if (DstIOperand.Rec->isSubClassOf("OperandWithDefaultOps")) {
+      DefaultOperands.insert(I);
+      NumDefaultOperands +=
+          DstIOperand.Rec->getValueAsDag("DefaultOps")->getNumArgs();
+    }
+  }
+  if (DstINumUses > Dst->getNumChildren() + DefaultOperands.size())
+    return failedImport("Insufficient operands supplied and default ops "
+                        "couldn't make up the shortfall");
+  if (DstINumUses < Dst->getNumChildren() + DefaultOperands.size())
+    return failedImport("Too many operands supplied");
+
   // Render the explicit uses.
-  for (unsigned i = 0, e = Dst->getNumChildren(); i != e; ++i) {
-    if (auto Error = importExplicitUseRenderer(DstMIBuilder, Dst->getChild(i),
-                                               InsnMatcher))
+  unsigned Child = 0;
+  for (unsigned I = 0; I != DstINumUses; ++I) {
+    // If we need to insert default ops here, then do so.
+    if (DefaultOperands.count(I)) {
+      const auto &DstIOperand = DstI.Operands[DstI.Operands.NumDefs + I];
+
+      DagInit *DefaultOps = DstIOperand.Rec->getValueAsDag("DefaultOps");
+      for (const auto *DefaultOp : DefaultOps->args()) {
+        // Look through ValueType operators.
+        if (const DagInit *DefaultDagOp = dyn_cast<DagInit>(DefaultOp)) {
+          if (const DefInit *DefaultDagOperator =
+                  dyn_cast<DefInit>(DefaultDagOp->getOperator())) {
+            if (DefaultDagOperator->getDef()->isSubClassOf("ValueType"))
+              DefaultOp = DefaultDagOp->getArg(0);
+          }
+        }
+
+        if (const DefInit *DefaultDefOp = dyn_cast<DefInit>(DefaultOp)) {
+          DstMIBuilder.addRenderer<AddRegisterRenderer>(DefaultDefOp->getDef());
+          continue;
+        }
+
+        if (const IntInit *DefaultIntOp = dyn_cast<IntInit>(DefaultOp)) {
+          DstMIBuilder.addRenderer<ImmRenderer>(DefaultIntOp->getValue());
+          continue;
+        }
+
+        return failedImport("Could not add default op");
+      }
+
+      continue;
+    }
+
+    if (auto Error = importExplicitUseRenderer(
+            DstMIBuilder, Dst->getChild(Child), InsnMatcher))
       return std::move(Error);
+    ++Child;
   }
 
   return DstMIBuilder;
 }
 
 Error GlobalISelEmitter::importImplicitDefRenderers(
     BuildMIAction &DstMIBuilder,
     const std::vector<Record *> &ImplicitDefs) const {