[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

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

242

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

Ok

242

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.

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)>;
 
 //===- 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:        }
 
 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:    }
 // 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:      MIB.setMemRefs(I.memoperands_begin(), I.memoperands_end());
+// 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 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)