[TableGen] More named sub-operands work.

[jyknight]

Summary

Commit a538d1f13a13 first added support for named sub-operands in
CodeEmitterGen. We now add a few more features to that, enabling
further target cleanups.

1. Adds support for handling an EncoderMethod in a sub-operand in

CodeEmitterGen. Previously, the specified encoder of a sub-operand was
ignored, and only the default used.

2. Adds support for sub-operands in DecoderEmitter, along with support

for tied sub-operands.

The changes to the decoder required a few minor tweaks to a few
targets, where existing brokeness was exposed. In order to keep this
patch small, I left FIXMEs which will be addressed in upcoming
patches. (Except MIPS16, since its object file emission/decoding is
totally broken).

[MaskRay]

llvm/lib/Target/ARM/ARMInstrFormats.td

276

OK. This is to avoid lib/Target/ARM/ARMInstrCDE.td:530:5: error: DecoderEmitter: operand "vp" uses MIOperandInfo with multiple ops, but doesn't have a custom decoder!

llvm/utils/TableGen/DecoderEmitter.cpp

1909

2215–2216

The comment appears outdated/unclear even before this patch. Could you reword it?

2258

OpTypeRec->isSubClassOf("Operand") is worth a comment.

2295

Delete braces for the single simple statement

2299

2300

Add a test/TableGen test for this error if it isn't too difficult?

[jyknight]

Address review comments.

[gbossu]

llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

6557

@jyknight I actually had a local patch which I planned to upstream in the next days, but that was made before your changes. That patch introduces an additional bit DecodeZeroBitOperand = false; flag in DAGOperand to allow targets to still get decoding callbacks for operands, even if they are encoded with zero bits. It came with a few changes for targets which have these "zero-bit" operands, like AVR, AArch64 or RiscV if I remember correctly.

It kept the default behavior of ignoring "zero-bit" operands when DecodeZeroBitOperand is false though. IIRC AMDGPU relies on this mechanism to purposely skip operands. Should I push this patch forward, or will this be superseded by some of your upcoming work by essentially always generating disassembler callbacks?

[jyknight]

llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

6557

Indeed I do have a work in progress change to fix this which I started back in November, but I haven't gotten back to yet. (I decided to complete the useDeprecatedPositionallyEncodedOperands cleanup first instead.)

I found that many targets do indeed currently "depend" on this behavior of ignoring no-bits operands -- but by-and-large only because they HAD to hardcode workarounds for the autogenerated decoder forgetting about them, in order to get correct behavior.

As with the other changes I've been making in this area, I intend to introduce the new behavior and control it with a Target option (e.g. useDeprecatedDecoderOperandSkipping), migrate all in-tree targets, and then delete the option.

[gbossu]

llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

6557

Thanks for the answer, I believe introducing useDeprecatedDecoderOperandSkipping is a better solution, as it forces targets in the right direction. Unless you don't intend to continue with your work in progress, I guess it doesn't make sense for me to push my patch forward? Please let me know if I can somehow assist though, as I attempted a very similar change.

llvm/lib/Target/ARC/ARCISelDAGToDAG.cpp

public:
 
   void Select(SDNode *N) override;
 
   // Complex Pattern Selectors.
   bool SelectFrameADDR_ri(SDValue Addr, SDValue &Base, SDValue &Offset);
   bool SelectAddrModeS9(SDValue Addr, SDValue &Base, SDValue &Offset);
   bool SelectAddrModeImm(SDValue Addr, SDValue &Base, SDValue &Offset);
   bool SelectAddrModeFar(SDValue Addr, SDValue &Base, SDValue &Offset);
-  bool SelectCMOVPred(SDValue N, SDValue &Pred, SDValue &Reg) {
-    const ConstantSDNode *CN = cast<ConstantSDNode>(N);
-    Pred = CurDAG->getTargetConstant(CN->getZExtValue(), SDLoc(N), MVT::i32);
-    Reg = CurDAG->getRegister(ARC::STATUS32, MVT::i32);
-    return true;
-  }
 
   StringRef getPassName() const override {
     return "ARC DAG->DAG Pattern Instruction Selection";
   }
 
 // Include the pieces autogenerated from the target description.
 #include "ARCGenDAGISel.inc"
 };

llvm/lib/Target/ARC/ARCInstrFormats.td

class F16_OP_U7<bit i, string asmstr> :
   F16_OP_IMM<0b11100, (outs GPR32:$b), (ins immU<7>:$u7), asmstr> {
 
   bits<7> u7;
   let Inst{7} = i;
   let Inst{6-0} = u7;
 }
 
 // Special types for different instruction operands.
-def cmovpred : Operand<i32>, PredicateOp,
-               ComplexPattern<i32, 2, "SelectCMOVPred"> {
-  let MIOperandInfo = (ops i32imm, i32imm);
-  let PrintMethod = "printPredicateOperand";
-}
-
 def ccond : Operand<i32> {
   let MIOperandInfo = (ops i32imm);
   let PrintMethod = "printPredicateOperand";
 }
 
 def brccond : Operand<i32> {
   let MIOperandInfo = (ops i32imm);
   let PrintMethod = "printBRCCPredicateOperand";

llvm/lib/Target/ARC/ARCInstrInfo.td

def MOV_f_ru6 : F32_DOP_RU6<0b00100, 0b001010, 1,
   let Defs = [STATUS32];
 }
 
 def cmov : PatFrag<(ops node:$op1, node:$op2, node:$cc),
                    (ARCcmov $op1, $op2, $cc)>;
 let Uses = [STATUS32], isAsCheapAsAMove = 1, isPredicable=1,
     isReMaterializable = 0, Constraints = "$B = $B2" in {
   def MOV_cc : F32_DOP_CC_RR<0b00100, 0b001010, 0,
-                 (outs GPR32:$B), (ins GPR32:$C, GPR32:$B2, cmovpred:$cc),
+                 (outs GPR32:$B), (ins GPR32:$C, GPR32:$B2, CCOp:$cc),
                  "mov.$cc\t$B, $C",
-                 [(set GPR32:$B, (cmov i32:$C, i32:$B2, cmovpred:$cc))]>;
+                 [(set GPR32:$B, (cmov i32:$C, i32:$B2, timm:$cc))]>;
 
   def MOV_cc_ru6 : F32_DOP_CC_RU6<0b00100, 0b001010, 0,
                  (outs GPR32:$B), (ins u6:$C, CCOp:$cc, GPR32:$B2),
                  "mov.$cc\t$B, $C", []>;
 
   def MOV_cc_f_ru6 : F32_DOP_CC_RU6<0b00100, 0b001010, 1,
                     (outs GPR32:$B), (ins u6:$C, CCOp:$cc, GPR32:$B2),
                     "mov.$cc.f\t$B, $C", []> {

llvm/lib/Target/ARM/ARMInstrFormats.td

def vpred_r : vpred_ops<(ops (v4i32 undef_tied_input)), (ops MQPR:$inactive)> {
   let OperandType = "OPERAND_VPRED_R";
   let DecoderMethod = "DecodeVpredROperand";
   let vpred_constraint = ",$Qd = $vp.inactive";
 }
 
 def vpred_n : vpred_ops<(ops), (ops)> {
   let ParserMatchClass = VPTPredNOperand;
   let OperandType = "OPERAND_VPRED_N";
+  let DecoderMethod = "DecodeVpredNOperand";
   let vpred_constraint = "";
 }
 
 // ARM special operands for disassembly only.
 //
 def SetEndAsmOperand : ImmAsmOperand<0,1> {
   let Name = "SetEndImm";
   let ParserMethod = "parseSetEndImm";

llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

 static DecodeStatus DecodeVSCCLRM(MCInst &Inst, unsigned Insn, uint64_t Address,
                                   const MCDisassembler *Decoder);
 static DecodeStatus DecodeVPTMaskOperand(MCInst &Inst, unsigned Val,
                                          uint64_t Address,
                                          const MCDisassembler *Decoder);
 static DecodeStatus DecodeVpredROperand(MCInst &Inst, unsigned Val,
                                         uint64_t Address,
                                         const MCDisassembler *Decoder);
+static DecodeStatus DecodeVpredNOperand(MCInst &Inst, unsigned Val,
+                                        uint64_t Address,
+                                        const MCDisassembler *Decoder);
 static DecodeStatus
 DecodeRestrictedIPredicateOperand(MCInst &Inst, unsigned Val, uint64_t Address,
                                   const MCDisassembler *Decoder);
 static DecodeStatus
 DecodeRestrictedSPredicateOperand(MCInst &Inst, unsigned Val, uint64_t Address,
                                   const MCDisassembler *Decoder);
 static DecodeStatus
 DecodeRestrictedUPredicateOperand(MCInst &Inst, unsigned Val, uint64_t Address,
static DecodeStatus DecodeVpredROperand(MCInst &Inst, unsigned RegNo,
   // to the MCInst at this stage, because AddThumbPredicate will do it
   // later, and will infer the register number from the TIED_TO
   // constraint. So this is a deliberately empty decoder method that
   // will inhibit the auto-generated disassembly code from adding an
   // operand at all.
   return MCDisassembler::Success;
 }
 
+[[maybe_unused]] static DecodeStatus
+DecodeVpredNOperand(MCInst &Inst, unsigned RegNo, uint64_t Address,
+                    const MCDisassembler *Decoder) {
+  // Similar to above, we want to ensure that no operands are added for the
+  // vpred operands. (This is marked "maybe_unused" for the moment; because
+  // DecoderEmitter currently (wrongly) omits operands with no instruction bits,
+  // the decoder doesn't actually call it yet. That will be addressed in a
+  // future change.)
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus
 DecodeRestrictedIPredicateOperand(MCInst &Inst, unsigned Val, uint64_t Address,
                                   const MCDisassembler *Decoder) {
   Inst.addOperand(MCOperand::createImm((Val & 0x1) == 0 ? ARMCC::EQ : ARMCC::NE));
   return MCDisassembler::Success;
 }
 
 static DecodeStatus

llvm/lib/Target/Mips/Disassembler/MipsDisassembler.cpp

 static DecodeStatus DecodeMovePRegPair(MCInst &Inst, unsigned RegPair,
                                        uint64_t Address,
                                        const MCDisassembler *Decoder);
 
 static DecodeStatus DecodeMovePOperands(MCInst &Inst, unsigned Insn,
                                         uint64_t Address,
                                         const MCDisassembler *Decoder);
 
+static DecodeStatus DecodeFIXMEInstruction(MCInst &Inst, unsigned Insn,
+                                           uint64_t Address,
+                                           const MCDisassembler *Decoder);
+
 static MCDisassembler *createMipsDisassembler(
                        const Target &T,
                        const MCSubtargetInfo &STI,
                        MCContext &Ctx) {
   return new MipsDisassembler(STI, Ctx, true);
 }
 
 static MCDisassembler *createMipselDisassembler(
if (HasRs)
     MCOperand::createReg(getReg(Decoder, Mips::GPR32RegClassID, Rs)));
   MI.addOperand(
     MCOperand::createReg(getReg(Decoder, Mips::GPR32RegClassID, Rt)));
 
   MI.addOperand(MCOperand::createImm(Imm));
 
   return MCDisassembler::Success;
 }
+
+// This instruction does not have a working decoder, and needs to be
+// fixed. This "fixme" function was introduced to keep the backend compiling,
+// while making changes to tablegen code.
+static DecodeStatus DecodeFIXMEInstruction(MCInst &Inst, unsigned Insn,
+                                           uint64_t Address,
+                                           const MCDisassembler *Decoder) {
+  return MCDisassembler::Fail;
+}

llvm/lib/Target/Mips/Mips16InstrInfo.td

def AddiuRxRxImm16: F2RI16_ins<0b01001, "addiu", IIM16Alu>,
   ArithLogic16Defs<0> {
   let AddedComplexity = 5;
 }
 def AddiuRxRxImmX16: FEXT_2RI16_ins<0b01001, "addiu", IIM16Alu>,
   ArithLogic16Defs<0> {
   let isCodeGenOnly = 1;
 }
 
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def AddiuRxRyOffMemX16:
   FEXT_RRI_A16_mem_ins<0, "addiu", mem16_ea, IIM16Alu>;
 
 //
 
 // Format: ADDIU rx, pc, immediate MIPS16e
 // Purpose: Add Immediate Unsigned Word (3-Operand, PC-Relative, Extended)
 // To add a constant to the program counter.

 def LwRxRyOffMemX16: FEXT_RRI16_mem_ins<0b10011, "lw", mem16, II_LW>, MayLoad{
   let isCodeGenOnly = 1;
 }
 
 // Format: LW rx, offset(sp) MIPS16e
 // Purpose: Load Word (SP-Relative, Extended)
 // To load an SP-relative word from memory as a signed value.
 //
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def LwRxSpImmX16: FEXT_RRI16_mem_ins<0b10010, "lw", mem16sp, II_LW>, MayLoad;
 
 def LwRxPcTcp16: FRI16_TCP_ins<0b10110, "lw", II_LW>, MayLoad;
 
 def LwRxPcTcpX16: FEXT_RI16_TCP_ins<0b10110, "lw", II_LW>, MayLoad;
 //
 // Format: MOVE r32, rz MIPS16e
 // Purpose: Move
def SaveX16:
   let Uses = [SP];
   let Defs = [SP];
 }
 //
 // Format: SB ry, offset(rx) MIPS16e
 // Purpose: Store Byte (Extended)
 // To store a byte to memory.
 //
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def SbRxRyOffMemX16:
   FEXT_RRI16_mem2_ins<0b11000, "sb", mem16, II_SB>, MayStore;
 
 //
 // Format: SEB rx MIPS16e
 // Purpose: Sign-Extend Byte
 // Sign-extend least significant byte in register rx.
 //

 //
 def SelTBtneZSltiu: SeliT<"btnez", "sltiu">;
 //
 //
 // Format: SH ry, offset(rx) MIPS16e
 // Purpose: Store Halfword (Extended)
 // To store a halfword to memory.
 //
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def ShRxRyOffMemX16:
   FEXT_RRI16_mem2_ins<0b11001, "sh", mem16, II_SH>, MayStore;
 
 //
 // Format: SLL rx, ry, sa MIPS16e
 // Purpose: Shift Word Left Logical (Extended)
 // To execute a left-shift of a word by a fixed number of bits-0 to 31 bits.
 //

 //
 def SubuRxRyRz16: FRRR16_ins<0b11, "subu", IIM16Alu>, ArithLogic16Defs<0>;
 
 //
 // Format: SW ry, offset(rx) MIPS16e
 // Purpose: Store Word (Extended)
 // To store a word to memory.
 //
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def SwRxRyOffMemX16: FEXT_RRI16_mem2_ins<0b11011, "sw", mem16, II_SW>, MayStore;
 
 //
 // Format: SW rx, offset(sp) MIPS16e
 // Purpose: Store Word rx (SP-Relative)
 // To store an SP-relative word to memory.
 //
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def SwRxSpImmX16: FEXT_RRI16_mem2_ins<0b11010, "sw", mem16sp, II_SW>, MayStore;
 
 //
 //
 // Format: XOR rx, ry MIPS16e
 // Purpose: Xor
 // To do a bitwise logical XOR.
 //

llvm/lib/Target/Sparc/Disassembler/SparcDisassembler.cpp

 static DecodeStatus DecodeJMPL(MCInst &Inst, unsigned insn, uint64_t Address,
                                const MCDisassembler *Decoder);
 static DecodeStatus DecodeReturn(MCInst &MI, unsigned insn, uint64_t Address,
                                  const MCDisassembler *Decoder);
 static DecodeStatus DecodeSWAP(MCInst &Inst, unsigned insn, uint64_t Address,
                                const MCDisassembler *Decoder);
 static DecodeStatus DecodeTRAP(MCInst &Inst, unsigned insn, uint64_t Address,
                                const MCDisassembler *Decoder);
+static DecodeStatus DecodeFIXMEInstruction(MCInst &MI, unsigned insn,
+                                           uint64_t Address,
+                                           const MCDisassembler *Decoder);
 
 #include "SparcGenDisassemblerTables.inc"
 
 /// Read four bytes from the ArrayRef and return 32 bit word.
 static DecodeStatus readInstruction32(ArrayRef<uint8_t> Bytes, uint64_t Address,
                                       uint64_t &Size, uint32_t &Insn,
                                       bool IsLittleEndian) {
   // We want to read exactly 4 Bytes of data.
static DecodeStatus DecodeReturn(MCInst &MI, unsigned insn, uint64_t Address,
   else {
     status = DecodeIntRegsRegisterClass(MI, rs2, Address, Decoder);
     if (status != MCDisassembler::Success)
       return status;
   }
   return MCDisassembler::Success;
 }
 
+// This instruction does not have a working decoder, and needs to be
+// fixed. This "fixme" function was introduced to keep the backend compiling,
+// while making changes to tablegen code.
+static DecodeStatus DecodeFIXMEInstruction(MCInst &Inst, unsigned Insn,
+                                           uint64_t Address,
+                                           const MCDisassembler *Decoder) {
+  return MCDisassembler::Fail;
+}
+
 static DecodeStatus DecodeSWAP(MCInst &MI, unsigned insn, uint64_t Address,
                                const MCDisassembler *Decoder) {
 
   unsigned rd = fieldFromInstruction(insn, 25, 5);
   unsigned rs1 = fieldFromInstruction(insn, 14, 5);
   unsigned isImm = fieldFromInstruction(insn, 13, 1);
   bool hasAsi = fieldFromInstruction(insn, 23, 1); // (in op3 field)
   unsigned asi = fieldFromInstruction(insn, 5, 8);

llvm/lib/Target/Sparc/SparcInstrInfo.td

multiclass LoadA<string OpcStr, bits<6> Op3Val, bits<6> LoadAOp3Val,
                  InstrItinClass itin = NoItinerary> :
              Load<OpcStr, Op3Val, OpNode, RC, Ty, itin> {
   def Arr  : LoadASI<OpcStr, LoadAOp3Val, RC>;
 }
 
 // The LDSTUB instruction is supported for asm only.
 // It is unlikely that general-purpose code could make use of it.
 // CAS is preferred for sparc v9.
+let DecoderMethod = "DecodeFIXMEInstruction" in {
 def LDSTUBrr : F3_1<3, 0b001101, (outs IntRegs:$rd), (ins (MEMrr $rs1, $rs2):$addr),
                     "ldstub [$addr], $rd", []>;
 def LDSTUBri : F3_2<3, 0b001101, (outs IntRegs:$rd), (ins (MEMri $rs1, $simm13):$addr),
                     "ldstub [$addr], $rd", []>;
 def LDSTUBArr : F3_1_asi<3, 0b011101, (outs IntRegs:$rd),
                          (ins (MEMrr $rs1, $rs2):$addr, i8imm:$asi),
                          "ldstuba [$addr] $asi, $rd", []>;
+}
 
 // Store multiclass - Define both Reg+Reg/Reg+Imm patterns in one shot.
 multiclass Store<string OpcStr, bits<6> Op3Val, SDPatternOperator OpNode,
            RegisterClass RC, ValueType Ty, InstrItinClass itin = IIC_st> {
   def rr  : F3_1<3, Op3Val,
                  (outs), (ins (MEMrr $rs1, $rs2):$addr, RC:$rd),
                  !strconcat(OpcStr, " $rd, [$addr]"),
                  [(OpNode Ty:$rd, ADDRrr:$addr)],

 
 
 // Section B.31 - Unimplemented Instruction
 let rd = 0 in
   def UNIMP : F2_1<0b000, (outs), (ins i32imm:$imm22),
                   "unimp $imm22", []>;
 
 // Section B.32 - Flush Instruction Memory
-let rd = 0 in {
+let DecoderMethod = "DecodeFIXMEInstruction", rd = 0 in {
   def FLUSHrr : F3_1<2, 0b111011, (outs), (ins (MEMrr $rs1, $rs2):$addr),
                        "flush $addr", []>;
   def FLUSHri : F3_2<2, 0b111011, (outs), (ins (MEMri $rs1, $simm13):$addr),
                        "flush $addr", []>;
 
   // The no-arg FLUSH is only here for the benefit of the InstAlias
   // "flush", which cannot seem to use FLUSHrr, due to the inability
   // to construct a MEMrr with fixed G0 registers.

 
 let hasSideEffects = 1 in {
   defm TADDCCTV : F3_12np<"taddcctv", 0b100010>;
   defm TSUBCCTV : F3_12np<"tsubcctv", 0b100011>;
 }
 }
 
 // Section A.42 - Prefetch Data
-let Predicates = [HasV9] in {
+let DecoderMethod = "DecodeFIXMEInstruction", Predicates = [HasV9] in {
   def PREFETCHr : F3_1<3, 0b101101,
                    (outs), (ins (MEMrr $rs1, $rs2):$addr, shift_imm5:$rd),
                    "prefetch [$addr], $rd", []>;
   def PREFETCHi : F3_2<3, 0b101101,
                    (outs), (ins (MEMri $rs1, $simm13):$addr, shift_imm5:$rd),
                    "prefetch [$addr], $rd", []>;
 }
 

llvm/test/TableGen/FixedLenDecoderEmitter/MultiOps.td

+// RUN: not llvm-tblgen -gen-disassembler -I %p/../../../include %s 2>&1 | FileCheck %s --implicit-check-not=error:
+
+include "llvm/Target/Target.td"
+
+def ArchInstrInfo : InstrInfo { }
+
+def Arch : Target {
+  let InstructionSet = ArchInstrInfo;
+}
+
+def Reg : Register<"reg">;
+
+def Regs : RegisterClass<"foo", [i32], 0, (add Reg)>;
+
+def complex_nodec : Operand<i32> {
+  let MIOperandInfo = (ops Regs, Regs);
+}
+
+def complex_withdec : Operand<i32> {
+  let MIOperandInfo = (ops Regs, Regs);
+  let DecoderMethod = "DecodeComplex";
+}
+
+class ArchInstr : Instruction {
+  let Size = 1;
+  bits<8> Inst;
+}
+
+// This definition is broken in both directions:
+// 1. Uses a complex operand without a decoder, and without named sub-ops.
+// 2. Uses a complex operand with named sub-ops, but with a decoder as well.
+
+// CHECK: error: DecoderEmitter: operand "r1c" uses MIOperandInfo with multiple ops, but doesn't have a custom decoder!
+// CHECK: note: Dumping record for previous error:
+// CHECK: error: DecoderEmitter: operand "r1ab" has type "complex_withdec" with a custom DecoderMethod, but also named sub-operands.
+def foo1 : ArchInstr {
+  bits<2> r1a;
+  bits<2> r1b;
+  bits<2> r1c;
+
+  let Inst{1-0} = r1a;
+  let Inst{3-2} = r1b;
+  let Inst{5-4} = r1c;
+  let Inst{7-6} = 0b00;
+
+  let OutOperandList = (outs complex_nodec:$r1c);
+  let InOperandList = (ins (complex_withdec $r1a, $r1b):$r1ab);
+}
+
+// This definition has no errors.
+def foo2 : ArchInstr {
+  bits<2> r2a;
+  bits<2> r2b;
+  bits<2> r2c;
+
+  let Inst{1-0} = r2a;
+  let Inst{3-2} = r2b;
+  let Inst{5-4} = r2c;
+  let Inst{7-6} = 0b01;
+
+  let OutOperandList = (outs complex_withdec:$r2c);
+  let InOperandList = (ins (complex_nodec $r2a, $r2b):$r2ab);
+}

llvm/test/TableGen/RegisterEncoder.td

 def Reg : Register<"reg">;
 
 def RegClass : RegisterClass<"foo", [i32], 0, (add Reg)>;
 
 def RegOperand : RegisterOperand<RegClass> {
   let EncoderMethod = "barEncoder";
 }
 
-def foo : Instruction {
+def foo1 : Instruction {
   let Size = 1;
 
   let OutOperandList = (outs);
   let InOperandList = (ins RegOperand:$bar);
 
   bits<8> bar;
   bits<8> Inst = bar;
 }
 
-// CHECK: case ::foo: {
+// CHECK: case ::foo1: {
 // CHECK:   op = barEncoder
 // CHECK:   op &= UINT64_C(255);
 // CHECK:   Value |= op;
 // CHECK:   break;
 // CHECK: }
+
+
+// Also check that it works from a complex operand.
+
+def RegPair : Operand<i32> {
+  let MIOperandInfo = (ops RegOperand, RegOperand);
+}
+
+def foo2 : Instruction {
+  let Size = 1;
+
+  let OutOperandList = (outs);
+  let InOperandList = (ins (RegPair $r1, $r2):$r12);
+
+  bits<4> r1;
+  bits<4> r2;
+  bits<8> Inst;
+  let Inst{3-0} = r1;
+  let Inst{7-4} = r2;
+}
+
+// CHECK: case ::foo2: {
+// CHECK:   op = barEncoder
+// CHECK:   op &= UINT64_C(15);
+// CHECK:   Value |= op;
+// CHECK:   op = barEncoder
+// CHECK:   op &= UINT64_C(15);
+// CHECK:   Value |= op;
+// CHECK:   break;
+// CHECK: }

llvm/utils/TableGen/CodeEmitterGen.cpp

 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
-#include <cassert>
 #include <cstdint>
 #include <map>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 using namespace llvm;
bool CodeEmitterGen::addCodeToMergeInOperand(Record *R, BitsInit *BI,
   // order.
   unsigned OpIdx;
   std::pair<unsigned, unsigned> SubOp;
   if (CGI.Operands.hasSubOperandAlias(VarName, SubOp)) {
     OpIdx = CGI.Operands[SubOp.first].MIOperandNo + SubOp.second;
   } else if (CGI.Operands.hasOperandNamed(VarName, OpIdx)) {
     // Get the machine operand number for the indicated operand.
     OpIdx = CGI.Operands[OpIdx].MIOperandNo;
-    assert(!CGI.Operands.isFlatOperandNotEmitted(OpIdx) &&
-           "Explicitly used operand also marked as not emitted!");
   } else {
     // Fall back to positional lookup. By default, we now disable positional
     // lookup (and print an error, below), but even so, we'll do the lookup to
     // help print a helpful diagnostic message.
     //
     // TODO: When we remove useDeprecatedPositionallyEncodedOperands, delete all
     // this code, just leaving a "no operand named X in record Y" error.
 
if (!Target.getInstructionSet()->getValueAsBit(
                         R->getName() + " (would've used positional operand #" +
                         Twine(SO.first) + " ('" + OpName + "') sub-op #" +
                         Twine(SO.second) +
                         " with useDeprecatedPositionallyEncodedOperands=true)");
       return false;
     }
   }
 
+  if (CGI.Operands.isFlatOperandNotEmitted(OpIdx)) {
+    PrintError(R, "Operand " + VarName + " used but also marked as not emitted!");
+    return false;
+  }
+
   std::pair<unsigned, unsigned> SO = CGI.Operands.getSubOperandNumber(OpIdx);
-  std::string &EncoderMethodName = CGI.Operands[SO.first].EncoderMethodName;
+  std::string &EncoderMethodName =
+      CGI.Operands[SO.first].EncoderMethodNames[SO.second];
 
   if (UseAPInt)
     Case += "      op.clearAllBits();\n";
 
-  // If the source operand has a custom encoder, use it. This will
-  // get the encoding for all of the suboperands.
-  if (!EncoderMethodName.empty()) {
-    // A custom encoder has all of the information for the
-    // sub-operands, if there are more than one, so only
-    // query the encoder once per source operand.
-    if (SO.second == 0) {
-      Case += "      // op: " + VarName + "\n";
-      if (UseAPInt) {
-        Case += "      " + EncoderMethodName + "(MI, " + utostr(OpIdx);
-        Case += ", op";
-      } else {
-        Case += "      op = " + EncoderMethodName + "(MI, " + utostr(OpIdx);
-      }
-      Case += ", Fixups, STI);\n";
-    }
+
+  // If the source operand has a custom encoder, use it.
+  if (!EncoderMethodName.empty()) {
   } else {
-    Case += "      // op: " + VarName + "\n";
     if (UseAPInt) {
       Case += "      getMachineOpValue(MI, MI.getOperand(" + utostr(OpIdx) + ")";
       Case += ", op, Fixups, STI";
     } else {
       Case += "      op = getMachineOpValue(MI, MI.getOperand(" + utostr(OpIdx) + ")";
       Case += ", Fixups, STI";
     }
     Case += ");\n";

llvm/utils/TableGen/CodeGenInstruction.h

struct OperandInfo {
       /// Rec - The definition this operand is declared as.
       ///
       Record *Rec;
 
       /// Name - If this operand was assigned a symbolic name, this is it,
       /// otherwise, it's empty.
       std::string Name;
 
+      /// The names of sub-operands, if given, otherwise empty.
+      std::vector<std::string> SubOpNames;
+
       /// PrinterMethodName - The method used to print operands of this type in
       /// the asmprinter.
       std::string PrinterMethodName;
 
-      /// EncoderMethodName - The method used to get the machine operand value
-      /// for binary encoding. "getMachineOpValue" by default.
-      std::string EncoderMethodName;
+      /// The method used to get the machine operand value for binary
+      /// encoding, per sub-operand. If empty, uses "getMachineOpValue".
+      std::vector<std::string> EncoderMethodNames;
 
       /// OperandType - A value from MCOI::OperandType representing the type of
       /// the operand.
       std::string OperandType;
 
       /// MIOperandNo - Currently (this is meant to be phased out), some logical
       /// operands correspond to multiple MachineInstr operands.  In the X86
       /// target for example, one address operand is represented as 4
struct OperandInfo {
       /// up of multiple MI operands.
       DagInit *MIOperandInfo;
 
       /// Constraint info for this operand.  This operand can have pieces, so we
       /// track constraint info for each.
       std::vector<ConstraintInfo> Constraints;
 
       OperandInfo(Record *R, const std::string &N, const std::string &PMN,
-                  const std::string &EMN, const std::string &OT, unsigned MION,
-                  unsigned MINO, DagInit *MIOI)
-      : Rec(R), Name(N), PrinterMethodName(PMN), EncoderMethodName(EMN),
-        OperandType(OT), MIOperandNo(MION), MINumOperands(MINO),
-        MIOperandInfo(MIOI) {}
-
+                  const std::string &OT, unsigned MION, unsigned MINO,
+                  DagInit *MIOI)
+          : Rec(R), Name(N), SubOpNames(MINO), PrinterMethodName(PMN),
+            EncoderMethodNames(MINO), OperandType(OT), MIOperandNo(MION),
+            MINumOperands(MINO), DoNotEncode(MINO), MIOperandInfo(MIOI),
+            Constraints(MINO) {}
 
       /// getTiedOperand - If this operand is tied to another one, return the
       /// other operand number.  Otherwise, return -1.
       int getTiedRegister() const {
         for (unsigned j = 0, e = Constraints.size(); j != e; ++j) {
           const CGIOperandList::ConstraintInfo &CI = Constraints[j];
           if (CI.isTied()) return CI.getTiedOperand();
         }

llvm/utils/TableGen/CodeGenInstruction.cpp

for (unsigned i = 0; i != e; ++i){
     } else if (Rec->getName() == "variable_ops") {
       if (i < NumDefs)
         VariadicOuts = true;
       isVariadic = true;
       continue;
     } else if (Rec->isSubClassOf("RegisterClass")) {
       OperandType = "OPERAND_REGISTER";
     } else if (!Rec->isSubClassOf("PointerLikeRegClass") &&
-               !Rec->isSubClassOf("unknown_class"))
+               !Rec->isSubClassOf("unknown_class")) {
       PrintFatalError(R->getLoc(), "Unknown operand class '" + Rec->getName() +
                                        "' in '" + R->getName() +
                                        "' instruction!");
+    }
 
     // Check that the operand has a name and that it's unique.
     if (ArgName.empty())
       PrintFatalError(R->getLoc(), "In instruction '" + R->getName() +
                                        "', operand #" + Twine(i) +
                                        " has no name!");
     if (!OperandNames.insert(std::string(ArgName)).second)
       PrintFatalError(R->getLoc(),
                       "In instruction '" + R->getName() + "', operand #" +
                           Twine(i) +
                           " has the same name as a previous operand!");
 
+    OperandInfo &OpInfo = OperandList.emplace_back(
+        Rec, std::string(ArgName), std::string(PrintMethod),
+        OperandNamespace + "::" + OperandType, MIOperandNo, NumOps, MIOpInfo);
+
     if (SubArgDag) {
       if (SubArgDag->getNumArgs() != NumOps) {
         PrintFatalError(R->getLoc(), "In instruction '" + R->getName() +
                                          "', operand #" + Twine(i) + " has " +
                                          Twine(SubArgDag->getNumArgs()) +
                                          " sub-arg names, expected " +
                                          Twine(NumOps) + ".");
       }
if (SubArgDag) {
           PrintFatalError(R->getLoc(), "In instruction '" + R->getName() +
                                            "', operand #" + Twine(i) +
                                            " has no name!");
         if (!OperandNames.insert(std::string(SubArgName)).second)
           PrintFatalError(R->getLoc(),
                           "In instruction '" + R->getName() + "', operand #" +
                               Twine(i) + " sub-arg #" + Twine(j) +
                               " has the same name as a previous operand!");
+
+        if (auto MaybeEncoderMethod =
+                cast<DefInit>(MIOpInfo->getArg(j))
+                    ->getDef()
+                    ->getValueAsOptionalString("EncoderMethod")) {
+          OpInfo.EncoderMethodNames[j] = *MaybeEncoderMethod;
+        }
+
+        OpInfo.SubOpNames[j] = SubArgName;
         SubOpAliases[SubArgName] = std::make_pair(MIOperandNo, j);
       }
+    } else if (!EncoderMethod.empty()) {
+      // If we have no explicit sub-op dag, but have an top-level encoder
+      // method, the single encoder will multiple sub-ops, itself.
+      OpInfo.EncoderMethodNames[0] = EncoderMethod;
+      for (unsigned j = 1; j < NumOps; ++j)
+        OpInfo.DoNotEncode[j] = true;
     }
 
-    OperandList.emplace_back(
-        Rec, std::string(ArgName), std::string(PrintMethod),
-        std::string(EncoderMethod), OperandNamespace + "::" + OperandType,
-        MIOperandNo, NumOps, MIOpInfo);
     MIOperandNo += NumOps;
   }
 
   if (VariadicOuts)
     --NumDefs;
-
-  // Make sure the constraints list for each operand is large enough to hold
-  // constraint info, even if none is present.
-  for (OperandInfo &OpInfo : OperandList)
-    OpInfo.Constraints.resize(OpInfo.MINumOperands);
 }
 
 
 /// getOperandNamed - Return the index of the operand with the specified
 /// non-empty name.  If the instruction does not have an operand with the
 /// specified name, abort.
 ///
 unsigned CGIOperandList::getOperandNamed(StringRef Name) const {
while (true) {
     StringRef OpName;
     std::tie(OpName, DisableEncoding) = getToken(DisableEncoding, " ,\t");
     if (OpName.empty()) break;
 
     // Figure out which operand this is.
     std::pair<unsigned,unsigned> Op = ParseOperandName(OpName, false);
 
     // Mark the operand as not-to-be encoded.
-    if (Op.second >= OperandList[Op.first].DoNotEncode.size())
-      OperandList[Op.first].DoNotEncode.resize(Op.second+1);
     OperandList[Op.first].DoNotEncode[Op.second] = true;
   }
 
 }
 
 //===----------------------------------------------------------------------===//
 // CodeGenInstruction Implementation
 //===----------------------------------------------------------------------===//

llvm/utils/TableGen/DecoderEmitter.cpp

if (!OpName.empty()) {
         Operands[OpIdx].addField(CurrBitPos, EncodingSegment.BitWidth, Offset);
       }
     }
 
     CurrBitPos += EncodingSegment.BitWidth;
   }
 }
 
+static void debugDumpRecord(const Record &Rec) {
+  // Dump the record, so we can see what's going on...
+  std::string E;
+  raw_string_ostream S(E);
+  S << "Dumping record for previous error:\n";
+  S << Rec;
+  PrintNote(E);
+}
+
+/// For an operand field named OpName: populate OpInfo.InitValue with the
+/// constant-valued bit values, and OpInfo.Fields with the ranges of bits to
+/// insert from the decoded instruction.
 static void addOneOperandFields(const Record &EncodingDef, const BitsInit &Bits,
                                 std::map<std::string, std::string> &TiedNames,
                                 StringRef OpName, OperandInfo &OpInfo) {
   // Some bits of the operand may be required to be 1 depending on the
   // instruction's encoding. Collect those bits.
   if (const RecordVal *EncodedValue = EncodingDef.getValue(OpName))
     if (const BitsInit *OpBits = dyn_cast<BitsInit>(EncodedValue->getValue()))
       for (unsigned I = 0; I < OpBits->getNumBits(); ++I)
populateInstruction(CodeGenTarget &Target, const Record &EncodingDef,
   for (unsigned i = 0; i < In->getNumArgs(); ++i)
     InOutOperands.push_back(
         std::make_pair(In->getArg(i), In->getArgNameStr(i)));
 
   // Search for tied operands, so that we can correctly instantiate
   // operands that are not explicitly represented in the encoding.
   std::map<std::string, std::string> TiedNames;
   for (unsigned i = 0; i < CGI.Operands.size(); ++i) {
-    int tiedTo = CGI.Operands[i].getTiedRegister();
-    if (tiedTo != -1) {
+    auto &Op = CGI.Operands[i];
+    for (unsigned j = 0; j < Op.Constraints.size(); ++j) {
+      const CGIOperandList::ConstraintInfo &CI = Op.Constraints[j];
+      if (CI.isTied()) {
+        int tiedTo = CI.getTiedOperand();
         std::pair<unsigned, unsigned> SO =
             CGI.Operands.getSubOperandNumber(tiedTo);
-      TiedNames[std::string(InOutOperands[i].second)] =
-          std::string(InOutOperands[SO.first].second);
-      TiedNames[std::string(InOutOperands[SO.first].second)] =
-          std::string(InOutOperands[i].second);
+        std::string TiedName = CGI.Operands[SO.first].SubOpNames[SO.second];
+        if (TiedName.empty())
+          TiedName = CGI.Operands[SO.first].Name;
+        std::string MyName = Op.SubOpNames[j];
+        if (MyName.empty())
+          MyName = Op.Name;
+
+        TiedNames[MyName] = TiedName;
+        TiedNames[TiedName] = MyName;
+      }
     }
   }
 
   if (IsVarLenInst) {
     parseVarLenInstOperand(EncodingDef, InsnOperands, CGI);
   } else {
     std::map<std::string, std::vector<OperandInfo>> NumberedInsnOperands;
     std::set<std::string> NumberedInsnOperandsNoTie;
if (SupportPositionalDecoding) {
             break;
         }
 
         if (bi == Bits.getNumBits())
           continue;
 
         // Skip variables that correspond to explicitly-named operands.
         unsigned OpIdx;
-        if (CGI.Operands.hasOperandNamed(Vals[i].getName(), OpIdx))
+        std::pair<unsigned, unsigned> SubOp;
+        if (CGI.Operands.hasSubOperandAlias(Vals[i].getName(), SubOp) ||
+            CGI.Operands.hasOperandNamed(Vals[i].getName(), OpIdx))
           continue;
 
         // Get the bit range for this operand:
         unsigned bitStart = bi++, bitWidth = 1;
         for (; bi < Bits.getNumBits(); ++bi) {
           VarInit *Var = nullptr;
           VarBitInit *BI = dyn_cast<VarBitInit>(Bits.getBit(bi));
           if (BI)
for (const auto &Op : InOutOperands) {
               InsnOperands.push_back(
                   NumberedInsnOperands[TiedNames[std::string(OpName)]]
                                       [SO.second]);
             }
           }
           continue;
         }
       }
 
-      // At this point, we can locate the decoder field, but we need to know how
-      // to interpret it.  As a first step, require the target to provide
-      // callbacks for decoding register classes.
-
-      if (DagInit *Dag = dyn_cast<DagInit>(OpInit))
-        OpInit = Dag->getOperator();
-      OperandInfo OpInfo = getOpInfo(cast<DefInit>(OpInit)->getDef());
+      // We're ready to find the instruction encoding locations for this operand.
+
+      // First, find the operand type ("OpInit"), and sub-op names
+      // ("SubArgDag") if present.
+      DagInit *SubArgDag = dyn_cast<DagInit>(OpInit);
+      if (SubArgDag)
+        OpInit = SubArgDag->getOperator();
+      Record *OpTypeRec = cast<DefInit>(OpInit)->getDef();
+      // Lookup the sub-operands from the operand type record (note that only
+      // Operand subclasses have MIOperandInfo, see CodeGenInstruction.cpp).
+      DagInit *SubOps = OpTypeRec->isSubClassOf("Operand")
+                            ? OpTypeRec->getValueAsDag("MIOperandInfo")
+                            : nullptr;
+
+      // Lookup the decoder method and construct a new OperandInfo to hold our result.
+      OperandInfo OpInfo = getOpInfo(OpTypeRec);
+
+      // If we have named sub-operands...
+      if (SubArgDag) {
+        // Then there should not be a custom decoder specified on the top-level
+        // type.
+        if (!OpInfo.Decoder.empty()) {
+          PrintError(EncodingDef.getLoc(),
+                     "DecoderEmitter: operand \"" + OpName + "\" has type \"" +
+                         OpInit->getAsString() +
+                         "\" with a custom DecoderMethod, but also named "
+                         "sub-operands.");
+          continue;
+        }
+
+        // Decode each of the sub-ops separately.
+        assert(SubOps && SubArgDag->getNumArgs() == SubOps->getNumArgs());
+        for (unsigned i = 0; i < SubOps->getNumArgs(); ++i) {
+          StringRef SubOpName = SubArgDag->getArgNameStr(i);
+          OperandInfo SubOpInfo =
+              getOpInfo(cast<DefInit>(SubOps->getArg(i))->getDef());
+
+          addOneOperandFields(EncodingDef, Bits, TiedNames, SubOpName,
+                              SubOpInfo);
+          InsnOperands.push_back(SubOpInfo);
+        }
+        continue;
+      }
+
+      // Otherwise, if we have an operand with sub-operands, but they aren't
+      // named...
+      if (SubOps && OpInfo.Decoder.empty()) {
+        // If it's a single sub-operand, and no custom decoder, use the decoder
+        // from the one sub-operand.
+        if (SubOps->getNumArgs() == 1)
+          OpInfo = getOpInfo(cast<DefInit>(SubOps->getArg(0))->getDef());
+
+        // If we have multiple sub-ops, there'd better have a custom
+        // decoder. (Otherwise we don't know how to populate them properly...)
+        if (SubOps->getNumArgs() > 1) {
+          PrintError(EncodingDef.getLoc(),
+                     "DecoderEmitter: operand \"" + OpName +
+                         "\" uses MIOperandInfo with multiple ops, but doesn't "
+                         "have a custom decoder!");
+          debugDumpRecord(EncodingDef);
+          continue;
+        }
+      }
 
       addOneOperandFields(EncodingDef, Bits, TiedNames, OpName, OpInfo);
+      // FIXME: it should be an error not to find a definition for a given
+      // operand, rather than just failing to add it to the resulting
+      // instruction! (This is a longstanding bug, which will be addressed in an
+      // upcoming change.)
       if (OpInfo.numFields() > 0)
         InsnOperands.push_back(OpInfo);
     }
   }
   Operands[Opc] = InsnOperands;
 
 #if 0
   LLVM_DEBUG({
       // Dumps the instruction encoding bits.
       dumpBits(errs(), Bits);
 
       errs() << '\n';
 
       // Dumps the list of operand info.
       for (unsigned i = 0, e = CGI.Operands.size(); i != e; ++i) {
         const CGIOperandList::OperandInfo &Info = CGI.Operands[i];
         const std::string &OperandName = Info.Name;
         const Record &OperandDef = *Info.Rec;
 
         errs() << "\t" << OperandName << " (" << OperandDef.getName() << ")\n";
       }
     });
 #endif
 

llvm/utils/TableGen/VarLenCodeEmitterGen.cpp

if (isa<StringInit>(Val) || isa<DagInit>(Val)) {
         // Normalized: (slice <operand name>, <high bit>, <low bit>)
         const auto *DV = cast<DagInit>(Val);
         OperandName = cast<StringInit>(DV->getArg(0))->getValue();
         LoBit = static_cast<unsigned>(cast<IntInit>(DV->getArg(2))->getValue());
       }
 
       auto OpIdx = CGI.Operands.ParseOperandName(OperandName);
       unsigned FlatOpIdx = CGI.Operands.getFlattenedOperandNumber(OpIdx);
-      StringRef CustomEncoder = CGI.Operands[OpIdx.first].EncoderMethodName;
+      StringRef CustomEncoder =
+          CGI.Operands[OpIdx.first].EncoderMethodNames[OpIdx.second];
       if (ES.CustomEncoder.size())
         CustomEncoder = ES.CustomEncoder;
 
       SS.indent(6) << "Scratch.clearAllBits();\n";
       SS.indent(6) << "// op: " << OperandName.drop_front(1) << "\n";
       if (CustomEncoder.empty())
         SS.indent(6) << "getMachineOpValue(MI, MI.getOperand("
                      << utostr(FlatOpIdx) << ")";