[llvm] 372240d - [TableGen] More named sub-operands work.

James Y Knight via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 7 11:37:44 PST 2022


Author: James Y Knight
Date: 2022-12-07T14:37:08-05:00
New Revision: 372240dfe3d5a933d9585663e15c4b6173ff23c8

URL: https://github.com/llvm/llvm-project/commit/372240dfe3d5a933d9585663e15c4b6173ff23c8
DIFF: https://github.com/llvm/llvm-project/commit/372240dfe3d5a933d9585663e15c4b6173ff23c8.diff

LOG: [TableGen] More named sub-operands work.

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).

Differential Revision: https://reviews.llvm.org/D137653

Added: 
    llvm/test/TableGen/FixedLenDecoderEmitter/MultiOps.td

Modified: 
    llvm/lib/Target/ARC/ARCISelDAGToDAG.cpp
    llvm/lib/Target/ARC/ARCInstrFormats.td
    llvm/lib/Target/ARC/ARCInstrInfo.td
    llvm/lib/Target/ARM/ARMInstrFormats.td
    llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
    llvm/lib/Target/Mips/Disassembler/MipsDisassembler.cpp
    llvm/lib/Target/Mips/Mips16InstrInfo.td
    llvm/lib/Target/Sparc/Disassembler/SparcDisassembler.cpp
    llvm/lib/Target/Sparc/SparcInstrInfo.td
    llvm/test/TableGen/RegisterEncoder.td
    llvm/utils/TableGen/CodeEmitterGen.cpp
    llvm/utils/TableGen/CodeGenInstruction.cpp
    llvm/utils/TableGen/CodeGenInstruction.h
    llvm/utils/TableGen/DecoderEmitter.cpp
    llvm/utils/TableGen/VarLenCodeEmitterGen.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/ARC/ARCISelDAGToDAG.cpp b/llvm/lib/Target/ARC/ARCISelDAGToDAG.cpp
index f639c4e6f0ffe..40d9ae4603a97 100644
--- a/llvm/lib/Target/ARC/ARCISelDAGToDAG.cpp
+++ b/llvm/lib/Target/ARC/ARCISelDAGToDAG.cpp
@@ -48,12 +48,6 @@ class ARCDAGToDAGISel : public SelectionDAGISel {
   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";

diff  --git a/llvm/lib/Target/ARC/ARCInstrFormats.td b/llvm/lib/Target/ARC/ARCInstrFormats.td
index 2a109cc0f7649..d6d2eaffab19b 100644
--- a/llvm/lib/Target/ARC/ARCInstrFormats.td
+++ b/llvm/lib/Target/ARC/ARCInstrFormats.td
@@ -964,12 +964,6 @@ class F16_OP_U7<bit i, string asmstr> :
 }
 
 // Special types for 
diff erent 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";

diff  --git a/llvm/lib/Target/ARC/ARCInstrInfo.td b/llvm/lib/Target/ARC/ARCInstrInfo.td
index 4a0bc5cf74215..693bc8a78bc5b 100644
--- a/llvm/lib/Target/ARC/ARCInstrInfo.td
+++ b/llvm/lib/Target/ARC/ARCInstrInfo.td
@@ -395,9 +395,9 @@ def cmov : PatFrag<(ops node:$op1, node:$op2, node:$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),

diff  --git a/llvm/lib/Target/ARM/ARMInstrFormats.td b/llvm/lib/Target/ARM/ARMInstrFormats.td
index c9a2d21bec532..14e315534570d 100644
--- a/llvm/lib/Target/ARM/ARMInstrFormats.td
+++ b/llvm/lib/Target/ARM/ARMInstrFormats.td
@@ -273,6 +273,7 @@ def vpred_r : vpred_ops<(ops (v4i32 undef_tied_input)), (ops MQPR:$inactive)> {
 def vpred_n : vpred_ops<(ops), (ops)> {
   let ParserMatchClass = VPTPredNOperand;
   let OperandType = "OPERAND_VPRED_N";
+  let DecoderMethod = "DecodeVpredNOperand";
   let vpred_constraint = "";
 }
 

diff  --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index cb2a0c43ea421..b4e6b325341b1 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -629,6 +629,9 @@ static DecodeStatus DecodeVPTMaskOperand(MCInst &Inst, unsigned Val,
 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);
@@ -6544,6 +6547,17 @@ static DecodeStatus DecodeVpredROperand(MCInst &Inst, unsigned RegNo,
   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) {

diff  --git a/llvm/lib/Target/Mips/Disassembler/MipsDisassembler.cpp b/llvm/lib/Target/Mips/Disassembler/MipsDisassembler.cpp
index bf5f7b69f7fa8..fb92590350c75 100644
--- a/llvm/lib/Target/Mips/Disassembler/MipsDisassembler.cpp
+++ b/llvm/lib/Target/Mips/Disassembler/MipsDisassembler.cpp
@@ -486,6 +486,10 @@ 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,
@@ -2513,3 +2517,12 @@ static DecodeStatus DecodeBlezGroupBranchMMR6(MCInst &MI, InsnType insn,
 
   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;
+}

diff  --git a/llvm/lib/Target/Mips/Mips16InstrInfo.td b/llvm/lib/Target/Mips/Mips16InstrInfo.td
index f7d8bad2fb2dc..fb2a83dc90ea9 100644
--- a/llvm/lib/Target/Mips/Mips16InstrInfo.td
+++ b/llvm/lib/Target/Mips/Mips16InstrInfo.td
@@ -537,6 +537,7 @@ def AddiuRxRxImmX16: FEXT_2RI16_ins<0b01001, "addiu", IIM16Alu>,
   let isCodeGenOnly = 1;
 }
 
+let DecoderMethod = "DecodeFIXMEInstruction" in
 def AddiuRxRyOffMemX16:
   FEXT_RRI_A16_mem_ins<0, "addiu", mem16_ea, IIM16Alu>;
 
@@ -850,6 +851,7 @@ def LwRxRyOffMemX16: FEXT_RRI16_mem_ins<0b10011, "lw", mem16, II_LW>, MayLoad{
 // 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;
@@ -1006,6 +1008,7 @@ def SaveX16:
 // 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;
 
@@ -1144,6 +1147,7 @@ def SelTBtneZSltiu: SeliT<"btnez", "sltiu">;
 // 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;
 
@@ -1280,6 +1284,7 @@ def SubuRxRyRz16: FRRR16_ins<0b11, "subu", IIM16Alu>, ArithLogic16Defs<0>;
 // 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;
 
 //
@@ -1287,6 +1292,7 @@ def SwRxRyOffMemX16: FEXT_RRI16_mem2_ins<0b11011, "sw", mem16, II_SW>, MayStore;
 // 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;
 
 //

diff  --git a/llvm/lib/Target/Sparc/Disassembler/SparcDisassembler.cpp b/llvm/lib/Target/Sparc/Disassembler/SparcDisassembler.cpp
index ef1f623724f71..6a132edc9ec89 100644
--- a/llvm/lib/Target/Sparc/Disassembler/SparcDisassembler.cpp
+++ b/llvm/lib/Target/Sparc/Disassembler/SparcDisassembler.cpp
@@ -306,6 +306,9 @@ 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"
 
@@ -591,6 +594,15 @@ static DecodeStatus DecodeReturn(MCInst &MI, unsigned insn, uint64_t Address,
   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) {
 

diff  --git a/llvm/lib/Target/Sparc/SparcInstrInfo.td b/llvm/lib/Target/Sparc/SparcInstrInfo.td
index 43c17e1449ad3..c8c5ba0ed9a8a 100644
--- a/llvm/lib/Target/Sparc/SparcInstrInfo.td
+++ b/llvm/lib/Target/Sparc/SparcInstrInfo.td
@@ -419,6 +419,7 @@ multiclass LoadA<string OpcStr, bits<6> Op3Val, bits<6> LoadAOp3Val,
 // 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),
@@ -426,6 +427,7 @@ def LDSTUBri : F3_2<3, 0b001101, (outs IntRegs:$rd), (ins (MEMri $rs1, $simm13):
 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,
@@ -1195,7 +1197,7 @@ let rd = 0 in
                   "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),
@@ -1759,7 +1761,7 @@ let hasSideEffects = 1 in {
 }
 
 // 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", []>;

diff  --git a/llvm/test/TableGen/FixedLenDecoderEmitter/MultiOps.td b/llvm/test/TableGen/FixedLenDecoderEmitter/MultiOps.td
new file mode 100644
index 0000000000000..d7263b69dd8f4
--- /dev/null
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/MultiOps.td
@@ -0,0 +1,63 @@
+// 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);
+}

diff  --git a/llvm/test/TableGen/RegisterEncoder.td b/llvm/test/TableGen/RegisterEncoder.td
index b5fe2d206729c..3038eab42411c 100644
--- a/llvm/test/TableGen/RegisterEncoder.td
+++ b/llvm/test/TableGen/RegisterEncoder.td
@@ -18,7 +18,7 @@ def RegOperand : RegisterOperand<RegClass> {
   let EncoderMethod = "barEncoder";
 }
 
-def foo : Instruction {
+def foo1 : Instruction {
   let Size = 1;
 
   let OutOperandList = (outs);
@@ -28,9 +28,39 @@ def foo : Instruction {
   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: }

diff  --git a/llvm/utils/TableGen/CodeEmitterGen.cpp b/llvm/utils/TableGen/CodeEmitterGen.cpp
index f999e14aa76bd..aee150dd74742 100644
--- a/llvm/utils/TableGen/CodeEmitterGen.cpp
+++ b/llvm/utils/TableGen/CodeEmitterGen.cpp
@@ -25,7 +25,6 @@
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
-#include <cassert>
 #include <cstdint>
 #include <map>
 #include <set>
@@ -113,8 +112,6 @@ bool CodeEmitterGen::addCodeToMergeInOperand(Record *R, BitsInit *BI,
   } 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
@@ -164,30 +161,30 @@ bool CodeEmitterGen::addCodeToMergeInOperand(Record *R, BitsInit *BI,
     }
   }
 
+  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.
+  Case += "      // op: " + VarName + "\n";
+
+  // If the source operand has a custom encoder, use it.
   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 (UseAPInt) {
+      Case += "      " + EncoderMethodName + "(MI, " + utostr(OpIdx);
+      Case += ", op";
+    } else {
+      Case += "      op = " + EncoderMethodName + "(MI, " + utostr(OpIdx);
     }
+    Case += ", Fixups, STI);\n";
   } else {
-    Case += "      // op: " + VarName + "\n";
     if (UseAPInt) {
       Case += "      getMachineOpValue(MI, MI.getOperand(" + utostr(OpIdx) + ")";
       Case += ", op, Fixups, STI";

diff  --git a/llvm/utils/TableGen/CodeGenInstruction.cpp b/llvm/utils/TableGen/CodeGenInstruction.cpp
index ce70e2dac30a5..238c6a1b6ba8c 100644
--- a/llvm/utils/TableGen/CodeGenInstruction.cpp
+++ b/llvm/utils/TableGen/CodeGenInstruction.cpp
@@ -120,10 +120,11 @@ CGIOperandList::CGIOperandList(Record *R) : TheDef(R) {
     } 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())
@@ -136,6 +137,10 @@ CGIOperandList::CGIOperandList(Record *R) : TheDef(R) {
                           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() +
@@ -162,24 +167,30 @@ CGIOperandList::CGIOperandList(Record *R) : TheDef(R) {
                           "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);
 }
 
 
@@ -409,8 +420,6 @@ void CGIOperandList::ProcessDisableEncoding(StringRef DisableEncoding) {
     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;
   }
 

diff  --git a/llvm/utils/TableGen/CodeGenInstruction.h b/llvm/utils/TableGen/CodeGenInstruction.h
index 07a1bd276b191..72626caada566 100644
--- a/llvm/utils/TableGen/CodeGenInstruction.h
+++ b/llvm/utils/TableGen/CodeGenInstruction.h
@@ -84,13 +84,16 @@ template <typename T> class ArrayRef;
       /// 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.
@@ -119,12 +122,12 @@ template <typename T> class ArrayRef;
       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.

diff  --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 087875b7ef9ee..fef574b7b7f2d 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -1906,6 +1906,18 @@ void parseVarLenInstOperand(const Record &Def,
   }
 }
 
+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) {
@@ -1991,14 +2003,23 @@ populateInstruction(CodeGenTarget &Target, const Record &EncodingDef,
   // 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) {
-      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);
+    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);
+        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;
+      }
     }
   }
 
@@ -2054,7 +2075,9 @@ populateInstruction(CodeGenTarget &Target, const Record &EncodingDef,
 
         // 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:
@@ -2190,15 +2213,75 @@ populateInstruction(CodeGenTarget &Target, const Record &EncodingDef,
         }
       }
 
-      // 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.
+      // 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());
 
-      if (DagInit *Dag = dyn_cast<DagInit>(OpInit))
-        OpInit = Dag->getOperator();
-      OperandInfo OpInfo = getOpInfo(cast<DefInit>(OpInit)->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);
     }

diff  --git a/llvm/utils/TableGen/VarLenCodeEmitterGen.cpp b/llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
index a6bbe2f7ff374..81201229079c7 100644
--- a/llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
+++ b/llvm/utils/TableGen/VarLenCodeEmitterGen.cpp
@@ -449,7 +449,8 @@ std::string VarLenCodeEmitterGen::getInstructionCaseForEncoding(
 
       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;
 


        


More information about the llvm-commits mailing list