From ee8e7d9c59f98cd9abd5e598ac4ae73c14986dae Mon Sep 17 00:00:00 2001 From: Robert Imschweiler Date: Fri, 22 Aug 2025 08:21:32 -0500 Subject: [PATCH 1/5] [TableGen] Implement getOperandIdxName This is meant as the inverse of getNamedOperandIdx and returns the OpName for a given operand index for a given opcode. --- llvm/test/TableGen/get-named-operand-idx.td | 101 +++++++++----- llvm/utils/TableGen/InstrInfoEmitter.cpp | 138 ++++++++++++++------ 2 files changed, 171 insertions(+), 68 deletions(-) diff --git a/llvm/test/TableGen/get-named-operand-idx.td b/llvm/test/TableGen/get-named-operand-idx.td index ab23edd54c723..3d07ba1278a53 100644 --- a/llvm/test/TableGen/get-named-operand-idx.td +++ b/llvm/test/TableGen/get-named-operand-idx.td @@ -48,34 +48,75 @@ def InstD : InstBase { let UseNamedOperandTable = 0; } -// CHECK: #ifdef GET_INSTRINFO_OPERAND_ENUM -// CHECK: #undef GET_INSTRINFO_OPERAND_ENUM -// CHECK: namespace llvm::MyNamespace { -// CHECK: enum class OpName { -// CHECK: a = 0, -// CHECK: b = 1, -// CHECK: c = 2, -// CHECK: d = 3, -// CHECK: x = 4, -// CHECK: NUM_OPERAND_NAMES = 5, -// CHECK: }; // enum class OpName -// CHECK: } // end namespace llvm::MyNamespace -// CHECK: #endif //GET_INSTRINFO_OPERAND_ENUM +// CHECK-LABEL: #ifdef GET_INSTRINFO_OPERAND_ENUM +// CHECK-NEXT: #undef GET_INSTRINFO_OPERAND_ENUM +// CHECK-NEXT: namespace llvm::MyNamespace { +// CHECK-NEXT: enum class OpName { +// CHECK-NEXT: a = 0, +// CHECK-NEXT: b = 1, +// CHECK-NEXT: c = 2, +// CHECK-NEXT: d = 3, +// CHECK-NEXT: x = 4, +// CHECK-NEXT: NUM_OPERAND_NAMES = 5, +// CHECK-NEXT: }; // enum class OpName +// CHECK-EMPTY: +// CHECK-NEXT: LLVM_READONLY +// CHECK-NEXT: int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name); +// CHECK-NEXT: LLVM_READONLY +// CHECK-NEXT: OpName getOperandIdxName(uint16_t Opcode, int16_t Idx); +// CHECK-NEXT: } // end namespace llvm::MyNamespace +// CHECK-NEXT: #endif //GET_INSTRINFO_OPERAND_ENUM -// CHECK: #ifdef GET_INSTRINFO_NAMED_OPS -// CHECK: #undef GET_INSTRINFO_NAMED_OPS -// CHECK: namespace llvm::MyNamespace { -// CHECK: LLVM_READONLY -// CHECK: int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) { -// CHECK: assert(Name != OpName::NUM_OPERAND_NAMES); -// CHECK: static constexpr int8_t OperandMap[][5] = { -// CHECK: {0, 1, 2, -1, -1, }, -// CHECK: {-1, -1, -1, 0, 1, }, -// CHECK: }; -// CHECK: static constexpr uint8_t InstructionIndex[] = { -// CHECK: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -// CHECK: }; -// CHECK: return OperandMap[InstructionIndex[Opcode]][(unsigned)Name]; -// CHECK: } -// CHECK: } // end namespace llvm::MyNamespace -// CHECK: #endif //GET_INSTRINFO_NAMED_OPS +// CHECK-LABEL: #ifdef GET_INSTRINFO_NAMED_OPS +// CHECK-NEXT: #undef GET_INSTRINFO_NAMED_OPS +// CHECK-NEXT: namespace llvm::MyNamespace { +// CHECK-NEXT: LLVM_READONLY +// CHECK-NEXT: static uint8_t getInstructionIndexForOpLookup(uint16_t Opcode) { +// CHECK-NEXT: static constexpr uint8_t InstructionIndex[] = { +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, +// CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 2, 0, +// CHECK-NEXT: }; +// CHECK-NEXT: return InstructionIndex[Opcode]; +// CHECK-NEXT: } +// CHECK-NEXT: LLVM_READONLY +// CHECK-NEXT: int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) { +// CHECK-NEXT: assert(Name != OpName::NUM_OPERAND_NAMES); +// CHECK-NEXT: static constexpr int8_t OperandMap[][5] = { +// CHECK-NEXT: {-1, -1, -1, -1, -1, }, +// CHECK-NEXT: {0, 1, 2, -1, -1, }, +// CHECK-NEXT: {-1, -1, -1, 0, 1, }, +// CHECK-NEXT: }; +// CHECK-NEXT: unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode); +// CHECK-NEXT: return OperandMap[InstrIdx][(unsigned)Name]; +// CHECK-NEXT: } +// CHECK-NEXT: LLVM_READONLY +// CHECK-NEXT: OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) { +// CHECK-NEXT: assert(Idx >= 0 && Idx < 3); +// CHECK-NEXT: static constexpr uint8_t OperandMap[][3] = { +// CHECK-NEXT: {5, 5, 5, }, +// CHECK-NEXT: {0, 1, 2, }, +// CHECK-NEXT: {3, 4, 5, }, +// CHECK-NEXT: }; +// CHECK-NEXT: unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode); +// CHECK-NEXT: return (OpName) OperandMap[InstrIdx][(unsigned)Idx]; +// CHECK-NEXT: } +// CHECK-NEXT: } // end namespace llvm::MyNamespace +// CHECK-NEXT: #endif //GET_INSTRINFO_NAMED_OPS diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp index 6f72b51963f87..eedceb19e5783 100644 --- a/llvm/utils/TableGen/InstrInfoEmitter.cpp +++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp @@ -223,17 +223,104 @@ void InstrInfoEmitter::EmitOperandInfo(raw_ostream &OS, } } +static void emitGetInstructionIndexForOpLookup( + raw_ostream &OS, const MapVector, unsigned> &OperandMap, + ArrayRef InstructionIndex) { + StringRef Type = OperandMap.size() <= UINT8_MAX + 1 ? "uint8_t" : "uint16_t"; + OS << "LLVM_READONLY\n"; + OS << "static " << Type + << " getInstructionIndexForOpLookup(uint16_t Opcode) {\n"; + OS << " static constexpr " << Type << " InstructionIndex[] = {"; + for (auto [TableIndex, Entry] : enumerate(InstructionIndex)) + OS << (TableIndex % 16 == 0 ? "\n " : " ") << Entry << ','; + OS << "\n };\n"; + OS << " return InstructionIndex[Opcode];\n"; + OS << "}\n"; +} + +static void +emitGetNamedOperandIdx(raw_ostream &OS, StringRef Namespace, + const MapVector &OperandNameToID, + const MapVector, unsigned> &OperandMap, + unsigned MaxOperandNo, unsigned NumOperandNames) { + OS << "LLVM_READONLY\n"; + OS << "int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) {\n"; + OS << " assert(Name != OpName::NUM_OPERAND_NAMES);\n"; + if (!NumOperandNames) { + // There are no operands, so no need to emit anything + OS << " return -1;\n}\n"; + return; + } + assert(MaxOperandNo <= INT16_MAX && + "Too many operands for the operand name -> index table"); + StringRef Type = MaxOperandNo <= INT8_MAX ? "int8_t" : "int16_t"; + OS << " static constexpr " << Type << " OperandMap[][" << NumOperandNames + << "] = {\n"; + for (const auto &[OpList, _] : OperandMap) { + // Emit a row of the OperandMap table. + OS << " {"; + for (unsigned ID = 0; ID < NumOperandNames; ++ID) + OS << (ID < OpList.size() ? OpList[ID] : -1) << ", "; + OS << "},\n"; + } + OS << " };\n"; + + OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n"; + OS << " return OperandMap[InstrIdx][(unsigned)Name];\n"; + OS << "}\n"; +} + +static void +emitGetOperandIdxName(raw_ostream &OS, StringRef Namespace, + const MapVector &OperandNameToID, + const MapVector, unsigned> &OperandMap, + unsigned MaxOperandNo, unsigned NumOperandNames) { + OS << "LLVM_READONLY\n"; + OS << "OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) {\n"; + OS << " assert(Idx >= 0 && Idx < " << MaxOperandNo << ");\n"; + if (!MaxOperandNo) { + // There are no operands, so no need to emit anything + OS << " return -1;\n}\n"; + return; + } + assert(NumOperandNames <= UINT16_MAX && + "Too many operands for the operand index -> name table"); + StringRef Type = NumOperandNames <= UINT8_MAX ? "uint8_t" : "uint16_t"; + OS << " static constexpr " << Type << " OperandMap[][" << MaxOperandNo + << "] = {\n"; + for (const auto &[OpList, _] : OperandMap) { + SmallVector IDs(MaxOperandNo, NumOperandNames); + for (const auto &[ID, Idx] : enumerate(OpList)) { + if (Idx >= 0) + IDs[Idx] = ID; + } + // Emit a row of the OperandMap table. Map operand indices to enum values. + OS << " {"; + for (unsigned ID : IDs) + OS << ID << ", "; + OS << "},\n"; + } + OS << " };\n"; + + OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n"; + OS << " return (OpName) OperandMap[InstrIdx][(unsigned)Idx];\n"; + OS << "}\n"; +} + /// Generate a table and function for looking up the indices of operands by /// name. /// /// This code generates: /// - An enum in the llvm::TargetNamespace::OpName namespace, with one entry /// for each operand name. -/// - A 2-dimensional table called OperandMap for mapping OpName enum values to -/// operand indices. -/// - A function called getNamedOperandIdx(uint16_t Opcode, uint16_t NamedIdx) +/// - A 2-dimensional table for mapping OpName enum values to operand indices. +/// - A function called getNamedOperandIdx(uint16_t Opcode, OpName Name) /// for looking up the operand index for an instruction, given a value from /// OpName enum +/// - A 2-dimensional table for mapping operand indices to OpName enum values. +/// - A function called getOperandIdxName(uint16_t Opcode, int16_t Idx) +/// for looking up the OpName enum for an instruction, given the operand +/// index. This is the inverse of getNamedOperandIdx(). /// /// Fixed/Predefined instructions do not have UseNamedOperandTable enabled, so /// we can just skip them. Hence accept just the TargetInstructions. @@ -242,11 +329,6 @@ void InstrInfoEmitter::emitOperandNameMappings( ArrayRef TargetInstructions) { StringRef Namespace = Target.getInstNamespace(); - /// To facilitate assigning OpName enum values in the sorted alphabetical - /// order, we go through an indirection from OpName -> ID, and Enum -> ID. - /// This allows us to build the OpList and assign IDs to OpNames in a single - /// scan of the instructions below. - // Map of operand names to their ID. MapVector OperandNameToID; @@ -257,7 +339,7 @@ void InstrInfoEmitter::emitOperandNameMappings( /// instructions that have identical OpName -> Operand index mapping. MapVector, unsigned> OperandMap; - // Max operand index seen. + // Max operand index seen + 1. unsigned MaxOperandNo = 0; // Fixed/Predefined instructions do not have UseNamedOperandTable enabled, so @@ -277,7 +359,7 @@ void InstrInfoEmitter::emitOperandNameMappings( .first->second; OpList.resize(std::max((unsigned)OpList.size(), ID + 1), -1); OpList[ID] = Info.MIOperandNo; - MaxOperandNo = std::max(MaxOperandNo, Info.MIOperandNo); + MaxOperandNo = std::max(MaxOperandNo, Info.MIOperandNo + 1); } auto [It, Inserted] = OperandMap.try_emplace(std::move(OpList), OperandMap.size()); @@ -296,42 +378,22 @@ void InstrInfoEmitter::emitOperandNameMappings( OS << "}; // enum class OpName\n\n"; OS << "LLVM_READONLY\n"; OS << "int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name);\n"; + OS << "LLVM_READONLY\n"; + OS << "OpName getOperandIdxName(uint16_t Opcode, int16_t Idx);\n"; OS << "} // end namespace llvm::" << Namespace << '\n'; OS << "#endif //GET_INSTRINFO_OPERAND_ENUM\n\n"; OS << "#ifdef GET_INSTRINFO_NAMED_OPS\n"; OS << "#undef GET_INSTRINFO_NAMED_OPS\n"; OS << "namespace llvm::" << Namespace << " {\n"; - OS << "LLVM_READONLY\n"; - OS << "int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) {\n"; - OS << " assert(Name != OpName::NUM_OPERAND_NAMES);\n"; - if (NumOperandNames != 0) { - assert(MaxOperandNo <= INT16_MAX && - "Too many operands for the operand name -> index table"); - StringRef Type = MaxOperandNo <= INT8_MAX ? "int8_t" : "int16_t"; - OS << " static constexpr " << Type << " OperandMap[][" << NumOperandNames - << "] = {\n"; - for (const auto &[OpList, _] : OperandMap) { - // Emit a row of the OperandMap table. - OS << " {"; - for (unsigned ID = 0; ID < NumOperandNames; ++ID) - OS << (ID < OpList.size() ? OpList[ID] : -1) << ", "; - OS << "},\n"; - } - OS << " };\n"; - Type = OperandMap.size() <= UINT8_MAX + 1 ? "uint8_t" : "uint16_t"; - OS << " static constexpr " << Type << " InstructionIndex[] = {"; - for (auto [TableIndex, Entry] : enumerate(InstructionIndex)) - OS << (TableIndex % 16 == 0 ? "\n " : " ") << Entry << ','; - OS << "\n };\n"; + emitGetInstructionIndexForOpLookup(OS, OperandMap, InstructionIndex); + + emitGetNamedOperandIdx(OS, Namespace, OperandNameToID, OperandMap, + MaxOperandNo, NumOperandNames); + emitGetOperandIdxName(OS, Namespace, OperandNameToID, OperandMap, + MaxOperandNo, NumOperandNames); - OS << " return OperandMap[InstructionIndex[Opcode]][(unsigned)Name];\n"; - } else { - // There are no operands, so no need to emit anything - OS << " return -1;\n"; - } - OS << "}\n"; OS << "} // end namespace llvm::" << Namespace << '\n'; OS << "#endif //GET_INSTRINFO_NAMED_OPS\n\n"; } From 52d1cbab737af66901b357ad7150549005e11171 Mon Sep 17 00:00:00 2001 From: Robert Imschweiler Date: Sat, 23 Aug 2025 04:09:52 -0500 Subject: [PATCH 2/5] implement feedback --- llvm/test/TableGen/get-named-operand-idx.td | 27 +++---- .../Target/AMDGPU/AMDGPUUnitTests.cpp | 19 +++++ llvm/utils/TableGen/InstrInfoEmitter.cpp | 79 ++++++++++--------- 3 files changed, 72 insertions(+), 53 deletions(-) diff --git a/llvm/test/TableGen/get-named-operand-idx.td b/llvm/test/TableGen/get-named-operand-idx.td index 3d07ba1278a53..e6f6331cd9c48 100644 --- a/llvm/test/TableGen/get-named-operand-idx.td +++ b/llvm/test/TableGen/get-named-operand-idx.td @@ -51,7 +51,7 @@ def InstD : InstBase { // CHECK-LABEL: #ifdef GET_INSTRINFO_OPERAND_ENUM // CHECK-NEXT: #undef GET_INSTRINFO_OPERAND_ENUM // CHECK-NEXT: namespace llvm::MyNamespace { -// CHECK-NEXT: enum class OpName { +// CHECK-NEXT: enum class OpName : uint8_t { // CHECK-NEXT: a = 0, // CHECK-NEXT: b = 1, // CHECK-NEXT: c = 2, @@ -60,18 +60,15 @@ def InstD : InstBase { // CHECK-NEXT: NUM_OPERAND_NAMES = 5, // CHECK-NEXT: }; // enum class OpName // CHECK-EMPTY: -// CHECK-NEXT: LLVM_READONLY -// CHECK-NEXT: int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name); -// CHECK-NEXT: LLVM_READONLY -// CHECK-NEXT: OpName getOperandIdxName(uint16_t Opcode, int16_t Idx); +// CHECK-NEXT: LLVM_READONLY int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name); +// CHECK-NEXT: LLVM_READONLY OpName getOperandIdxName(uint16_t Opcode, int16_t Idx); // CHECK-NEXT: } // end namespace llvm::MyNamespace // CHECK-NEXT: #endif //GET_INSTRINFO_OPERAND_ENUM // CHECK-LABEL: #ifdef GET_INSTRINFO_NAMED_OPS // CHECK-NEXT: #undef GET_INSTRINFO_NAMED_OPS // CHECK-NEXT: namespace llvm::MyNamespace { -// CHECK-NEXT: LLVM_READONLY -// CHECK-NEXT: static uint8_t getInstructionIndexForOpLookup(uint16_t Opcode) { +// CHECK-NEXT: LLVM_READONLY static uint8_t getInstructionIndexForOpLookup(uint16_t Opcode) { // CHECK-NEXT: static constexpr uint8_t InstructionIndex[] = { // CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // CHECK-NEXT: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -96,8 +93,7 @@ def InstD : InstBase { // CHECK-NEXT: }; // CHECK-NEXT: return InstructionIndex[Opcode]; // CHECK-NEXT: } -// CHECK-NEXT: LLVM_READONLY -// CHECK-NEXT: int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) { +// CHECK-NEXT: LLVM_READONLY int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) { // CHECK-NEXT: assert(Name != OpName::NUM_OPERAND_NAMES); // CHECK-NEXT: static constexpr int8_t OperandMap[][5] = { // CHECK-NEXT: {-1, -1, -1, -1, -1, }, @@ -107,16 +103,15 @@ def InstD : InstBase { // CHECK-NEXT: unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode); // CHECK-NEXT: return OperandMap[InstrIdx][(unsigned)Name]; // CHECK-NEXT: } -// CHECK-NEXT: LLVM_READONLY -// CHECK-NEXT: OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) { +// CHECK-NEXT: LLVM_READONLY OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) { // CHECK-NEXT: assert(Idx >= 0 && Idx < 3); -// CHECK-NEXT: static constexpr uint8_t OperandMap[][3] = { -// CHECK-NEXT: {5, 5, 5, }, -// CHECK-NEXT: {0, 1, 2, }, -// CHECK-NEXT: {3, 4, 5, }, +// CHECK-NEXT: static constexpr OpName OperandMap[][3] = { +// CHECK-NEXT: {OpName::NUM_OPERAND_NAMES, OpName::NUM_OPERAND_NAMES, OpName::NUM_OPERAND_NAMES, }, +// CHECK-NEXT: {OpName::a, OpName::b, OpName::c, }, +// CHECK-NEXT: {OpName::d, OpName::x, OpName::NUM_OPERAND_NAMES, }, // CHECK-NEXT: }; // CHECK-NEXT: unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode); -// CHECK-NEXT: return (OpName) OperandMap[InstrIdx][(unsigned)Idx]; +// CHECK-NEXT: return OperandMap[InstrIdx][(unsigned)Idx]; // CHECK-NEXT: } // CHECK-NEXT: } // end namespace llvm::MyNamespace // CHECK-NEXT: #endif //GET_INSTRINFO_NAMED_OPS diff --git a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp index ac08501817340..cb30d8810c127 100644 --- a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp +++ b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp @@ -320,3 +320,22 @@ TEST(AMDGPU, TestReverseComposeSubRegIndices) { } } } + +TEST(AMDGPU, TestGetNamedOperandIdx) { + auto TM = createAMDGPUTargetMachine("amdgcn-amd-", "gfx900", ""); + if (!TM) + return; + const MCInstrInfo *MCII = TM->getMCInstrInfo(); + + for (unsigned Opcode = 0, E = MCII->getNumOpcodes(); Opcode != E; ++Opcode) { + const MCInstrDesc &Desc = MCII->get(Opcode); + for (unsigned Idx = 0; Idx < Desc.getNumOperands(); ++Idx) { + AMDGPU::OpName OpName = AMDGPU::getOperandIdxName(Opcode, Idx); + if (OpName == AMDGPU::OpName::NUM_OPERAND_NAMES) + continue; + int16_t RetrievedIdx = AMDGPU::getNamedOperandIdx(Opcode, OpName); + EXPECT_EQ(Idx, RetrievedIdx) + << "Opcode " << Opcode << " (" << MCII->getName(Opcode) << ")"; + } + } +} \ No newline at end of file diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp index eedceb19e5783..33a7d282f6afa 100644 --- a/llvm/utils/TableGen/InstrInfoEmitter.cpp +++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp @@ -227,24 +227,23 @@ static void emitGetInstructionIndexForOpLookup( raw_ostream &OS, const MapVector, unsigned> &OperandMap, ArrayRef InstructionIndex) { StringRef Type = OperandMap.size() <= UINT8_MAX + 1 ? "uint8_t" : "uint16_t"; - OS << "LLVM_READONLY\n"; - OS << "static " << Type - << " getInstructionIndexForOpLookup(uint16_t Opcode) {\n"; - OS << " static constexpr " << Type << " InstructionIndex[] = {"; + OS << "LLVM_READONLY static " << Type + << " getInstructionIndexForOpLookup(uint16_t Opcode) {\n" + " static constexpr " + << Type << " InstructionIndex[] = {"; for (auto [TableIndex, Entry] : enumerate(InstructionIndex)) OS << (TableIndex % 16 == 0 ? "\n " : " ") << Entry << ','; - OS << "\n };\n"; - OS << " return InstructionIndex[Opcode];\n"; - OS << "}\n"; + OS << "\n };\n" + " return InstructionIndex[Opcode];\n" + "}\n"; } static void -emitGetNamedOperandIdx(raw_ostream &OS, StringRef Namespace, - const MapVector &OperandNameToID, +emitGetNamedOperandIdx(raw_ostream &OS, const MapVector, unsigned> &OperandMap, unsigned MaxOperandNo, unsigned NumOperandNames) { - OS << "LLVM_READONLY\n"; - OS << "int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name) {\n"; + OS << "LLVM_READONLY int16_t getNamedOperandIdx(uint16_t Opcode, OpName " + "Name) {\n"; OS << " assert(Name != OpName::NUM_OPERAND_NAMES);\n"; if (!NumOperandNames) { // There are no operands, so no need to emit anything @@ -271,39 +270,40 @@ emitGetNamedOperandIdx(raw_ostream &OS, StringRef Namespace, } static void -emitGetOperandIdxName(raw_ostream &OS, StringRef Namespace, - const MapVector &OperandNameToID, +emitGetOperandIdxName(raw_ostream &OS, + MapVector OperandNameToID, const MapVector, unsigned> &OperandMap, - unsigned MaxOperandNo, unsigned NumOperandNames) { - OS << "LLVM_READONLY\n"; - OS << "OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) {\n"; - OS << " assert(Idx >= 0 && Idx < " << MaxOperandNo << ");\n"; - if (!MaxOperandNo) { + unsigned MaxNumOperands, unsigned NumOperandNames) { + OS << "LLVM_READONLY OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) " + "{\n"; + OS << " assert(Idx >= 0 && Idx < " << MaxNumOperands << ");\n"; + if (!MaxNumOperands) { // There are no operands, so no need to emit anything OS << " return -1;\n}\n"; return; } - assert(NumOperandNames <= UINT16_MAX && - "Too many operands for the operand index -> name table"); - StringRef Type = NumOperandNames <= UINT8_MAX ? "uint8_t" : "uint16_t"; - OS << " static constexpr " << Type << " OperandMap[][" << MaxOperandNo + OS << " static constexpr OpName OperandMap[][" << MaxNumOperands << "] = {\n"; for (const auto &[OpList, _] : OperandMap) { - SmallVector IDs(MaxOperandNo, NumOperandNames); + SmallVector IDs(MaxNumOperands, NumOperandNames); for (const auto &[ID, Idx] : enumerate(OpList)) { if (Idx >= 0) IDs[Idx] = ID; } // Emit a row of the OperandMap table. Map operand indices to enum values. OS << " {"; - for (unsigned ID : IDs) - OS << ID << ", "; + for (unsigned ID : IDs) { + if (ID == NumOperandNames) + OS << "OpName::NUM_OPERAND_NAMES, "; + else + OS << "OpName::" << OperandNameToID.getArrayRef()[ID].first << ", "; + } OS << "},\n"; } OS << " };\n"; OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n"; - OS << " return (OpName) OperandMap[InstrIdx][(unsigned)Idx];\n"; + OS << " return OperandMap[InstrIdx][(unsigned)Idx];\n"; OS << "}\n"; } @@ -339,7 +339,7 @@ void InstrInfoEmitter::emitOperandNameMappings( /// instructions that have identical OpName -> Operand index mapping. MapVector, unsigned> OperandMap; - // Max operand index seen + 1. + // Max operand index seen. unsigned MaxOperandNo = 0; // Fixed/Predefined instructions do not have UseNamedOperandTable enabled, so @@ -359,7 +359,7 @@ void InstrInfoEmitter::emitOperandNameMappings( .first->second; OpList.resize(std::max((unsigned)OpList.size(), ID + 1), -1); OpList[ID] = Info.MIOperandNo; - MaxOperandNo = std::max(MaxOperandNo, Info.MIOperandNo + 1); + MaxOperandNo = std::max(MaxOperandNo, Info.MIOperandNo); } auto [It, Inserted] = OperandMap.try_emplace(std::move(OpList), OperandMap.size()); @@ -367,19 +367,25 @@ void InstrInfoEmitter::emitOperandNameMappings( } const size_t NumOperandNames = OperandNameToID.size(); + const unsigned MaxNumOperands = MaxOperandNo + 1; OS << "#ifdef GET_INSTRINFO_OPERAND_ENUM\n"; OS << "#undef GET_INSTRINFO_OPERAND_ENUM\n"; OS << "namespace llvm::" << Namespace << " {\n"; - OS << "enum class OpName {\n"; + + assert(NumOperandNames <= UINT16_MAX && + "Too many operands for the operand index -> name table"); + StringRef EnumType = getMinimalTypeForRange(NumOperandNames); + OS << "enum class OpName : " << EnumType << " {\n"; for (const auto &[Op, I] : OperandNameToID) OS << " " << Op << " = " << I << ",\n"; OS << " NUM_OPERAND_NAMES = " << NumOperandNames << ",\n"; OS << "}; // enum class OpName\n\n"; - OS << "LLVM_READONLY\n"; - OS << "int16_t getNamedOperandIdx(uint16_t Opcode, OpName Name);\n"; - OS << "LLVM_READONLY\n"; - OS << "OpName getOperandIdxName(uint16_t Opcode, int16_t Idx);\n"; + + OS << "LLVM_READONLY int16_t getNamedOperandIdx(uint16_t Opcode, OpName " + "Name);\n"; + OS << "LLVM_READONLY OpName getOperandIdxName(uint16_t Opcode, int16_t " + "Idx);\n"; OS << "} // end namespace llvm::" << Namespace << '\n'; OS << "#endif //GET_INSTRINFO_OPERAND_ENUM\n\n"; @@ -389,10 +395,9 @@ void InstrInfoEmitter::emitOperandNameMappings( emitGetInstructionIndexForOpLookup(OS, OperandMap, InstructionIndex); - emitGetNamedOperandIdx(OS, Namespace, OperandNameToID, OperandMap, - MaxOperandNo, NumOperandNames); - emitGetOperandIdxName(OS, Namespace, OperandNameToID, OperandMap, - MaxOperandNo, NumOperandNames); + emitGetNamedOperandIdx(OS, OperandMap, MaxOperandNo, NumOperandNames); + emitGetOperandIdxName(OS, OperandNameToID, OperandMap, MaxNumOperands, + NumOperandNames); OS << "} // end namespace llvm::" << Namespace << '\n'; OS << "#endif //GET_INSTRINFO_NAMED_OPS\n\n"; From 3d2457a6a503c0aa78dbab9f62656b63538a19c6 Mon Sep 17 00:00:00 2001 From: Robert Imschweiler Date: Sat, 23 Aug 2025 14:29:47 +0200 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Matt Arsenault --- llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp | 6 +++--- llvm/utils/TableGen/InstrInfoEmitter.cpp | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp index cb30d8810c127..4a028b9f63c60 100644 --- a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp +++ b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp @@ -322,7 +322,7 @@ TEST(AMDGPU, TestReverseComposeSubRegIndices) { } TEST(AMDGPU, TestGetNamedOperandIdx) { - auto TM = createAMDGPUTargetMachine("amdgcn-amd-", "gfx900", ""); + const TargetMachine *TM = createAMDGPUTargetMachine("amdgcn-amd-", "gfx900", ""); if (!TM) return; const MCInstrInfo *MCII = TM->getMCInstrInfo(); @@ -335,7 +335,7 @@ TEST(AMDGPU, TestGetNamedOperandIdx) { continue; int16_t RetrievedIdx = AMDGPU::getNamedOperandIdx(Opcode, OpName); EXPECT_EQ(Idx, RetrievedIdx) - << "Opcode " << Opcode << " (" << MCII->getName(Opcode) << ")"; + << "Opcode " << Opcode << " (" << MCII->getName(Opcode) << ')'; } } -} \ No newline at end of file +} diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp index 33a7d282f6afa..d2786596c5843 100644 --- a/llvm/utils/TableGen/InstrInfoEmitter.cpp +++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp @@ -264,9 +264,9 @@ emitGetNamedOperandIdx(raw_ostream &OS, } OS << " };\n"; - OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n"; - OS << " return OperandMap[InstrIdx][(unsigned)Name];\n"; - OS << "}\n"; + OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n" + " return OperandMap[InstrIdx][(unsigned)Name];\n" + "}\n"; } static void @@ -302,9 +302,9 @@ emitGetOperandIdxName(raw_ostream &OS, } OS << " };\n"; - OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n"; - OS << " return OperandMap[InstrIdx][(unsigned)Idx];\n"; - OS << "}\n"; + OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n" + " return OperandMap[InstrIdx][(unsigned)Idx];\n" + "}\n"; } /// Generate a table and function for looking up the indices of operands by From 4f81f8b946dd8187edf15ae8cd9c6661719450e1 Mon Sep 17 00:00:00 2001 From: Robert Imschweiler Date: Sat, 23 Aug 2025 07:35:00 -0500 Subject: [PATCH 4/5] fix formatting --- llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp | 3 ++- llvm/utils/TableGen/InstrInfoEmitter.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp index 4a028b9f63c60..97a4b1f2fef87 100644 --- a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp +++ b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp @@ -322,7 +322,8 @@ TEST(AMDGPU, TestReverseComposeSubRegIndices) { } TEST(AMDGPU, TestGetNamedOperandIdx) { - const TargetMachine *TM = createAMDGPUTargetMachine("amdgcn-amd-", "gfx900", ""); + const TargetMachine *TM = + createAMDGPUTargetMachine("amdgcn-amd-", "gfx900", ""); if (!TM) return; const MCInstrInfo *MCII = TM->getMCInstrInfo(); diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp index d2786596c5843..26d93fc13c9ba 100644 --- a/llvm/utils/TableGen/InstrInfoEmitter.cpp +++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp @@ -265,8 +265,8 @@ emitGetNamedOperandIdx(raw_ostream &OS, OS << " };\n"; OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n" - " return OperandMap[InstrIdx][(unsigned)Name];\n" - "}\n"; + " return OperandMap[InstrIdx][(unsigned)Name];\n" + "}\n"; } static void @@ -303,8 +303,8 @@ emitGetOperandIdxName(raw_ostream &OS, OS << " };\n"; OS << " unsigned InstrIdx = getInstructionIndexForOpLookup(Opcode);\n" - " return OperandMap[InstrIdx][(unsigned)Idx];\n" - "}\n"; + " return OperandMap[InstrIdx][(unsigned)Idx];\n" + "}\n"; } /// Generate a table and function for looking up the indices of operands by From 6a507478dce562d1582966703ef29b6f6d6280de Mon Sep 17 00:00:00 2001 From: Robert Imschweiler Date: Sat, 23 Aug 2025 07:40:35 -0500 Subject: [PATCH 5/5] fix targetmachine ptr type --- llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp index 97a4b1f2fef87..d01d808f988ba 100644 --- a/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp +++ b/llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp @@ -322,7 +322,7 @@ TEST(AMDGPU, TestReverseComposeSubRegIndices) { } TEST(AMDGPU, TestGetNamedOperandIdx) { - const TargetMachine *TM = + std::unique_ptr TM = createAMDGPUTargetMachine("amdgcn-amd-", "gfx900", ""); if (!TM) return;