Skip to content

[TableGen] Implement getOperandIdxName #154944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ro-i merged 5 commits into from
Aug 25, 2025
Merged

[TableGen] Implement getOperandIdxName #154944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ee8e7d9
[TableGen] Implement getOperandIdxName
ro-i Aug 22, 2025
52d1cba
implement feedback
ro-i Aug 23, 2025
3d2457a
Apply suggestions from code review
ro-i Aug 23, 2025
4f81f8b
fix formatting
ro-i Aug 23, 2025
6a50747
fix targetmachine ptr type
ro-i Aug 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 66 additions & 30 deletions llvm/test/TableGen/get-named-operand-idx.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,34 +48,70 @@ 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 : uint8_t {
// 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 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: #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 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 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 OpName getOperandIdxName(uint16_t Opcode, int16_t Idx) {
// CHECK-NEXT: assert(Idx >= 0 && Idx < 3);
// 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 OperandMap[InstrIdx][(unsigned)Idx];
// CHECK-NEXT: }
// CHECK-NEXT: } // end namespace llvm::MyNamespace
// CHECK-NEXT: #endif //GET_INSTRINFO_NAMED_OPS
20 changes: 20 additions & 0 deletions llvm/unittests/Target/AMDGPU/AMDGPUUnitTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,23 @@ TEST(AMDGPU, TestReverseComposeSubRegIndices) {
}
}
}

TEST(AMDGPU, TestGetNamedOperandIdx) {
std::unique_ptr<const GCNTargetMachine> 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)
Copy link
Collaborator

@mikaelholmen mikaelholmen Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ro-i ,

Warning on this line:

../../third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned int' and 'const short' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
../../third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned int, short>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
../unittests/Target/AMDGPU/AMDGPUUnitTests.cpp:338:7: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned int, short, nullptr>' requested here
  338 |       EXPECT_EQ(Idx, RetrievedIdx)
      |       ^
``

Copy link
Contributor Author

@ro-i ro-i Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#155235

<< "Opcode " << Opcode << " (" << MCII->getName(Opcode) << ')';
}
}
}
145 changes: 106 additions & 39 deletions llvm/utils/TableGen/InstrInfoEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,104 @@ void InstrInfoEmitter::EmitOperandInfo(raw_ostream &OS,
}
}

static void emitGetInstructionIndexForOpLookup(
raw_ostream &OS, const MapVector<SmallVector<int>, unsigned> &OperandMap,
ArrayRef<unsigned> InstructionIndex) {
StringRef Type = OperandMap.size() <= UINT8_MAX + 1 ? "uint8_t" : "uint16_t";
OS << "LLVM_READONLY static " << Type
Copy link
Contributor

@arsenm arsenm Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OS << "LLVM_READONLY static " << Type
OS << "LLVM_READONLY static " << Type

I think this technically qualifies for readnone, but the optimizer will take care of it anyway

<< " getInstructionIndexForOpLookup(uint16_t Opcode) {\n"
" static constexpr "
<< Type << " InstructionIndex[] = {";
for (auto [TableIndex, Entry] : enumerate(InstructionIndex))
OS << (TableIndex % 16 == 0 ? "\n " : " ") << Entry << ',';
OS << "\n };\n"
" return InstructionIndex[Opcode];\n"
"}\n";
}

static void
emitGetNamedOperandIdx(raw_ostream &OS,
const MapVector<SmallVector<int>, unsigned> &OperandMap,
unsigned MaxOperandNo, unsigned NumOperandNames) {
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
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) {
Copy link
Contributor

@shiltian shiltian Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is _ a C++ thing? I'm not sure if you could use that to ignore unused variable warning.

Copy link
Contributor Author

@ro-i ro-i Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is currently no warning intended for unused member of structured bindings: #58953
This might change in the future, and then, _ will probably be supported to explicitly ignore structured binding members. Currently, it's just a visual helper for the reader.

Copy link
Contributor

@jurahul jurahul Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this idiom is already used heavily in LLVM code. _ is a valid variable name in C++ I think, which is what this uses.

Copy link
Contributor

@arsenm arsenm Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC there's a proposal to make this blessed syntax in a future C++

Copy link
Contributor

@shiltian shiltian Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was one time I indeed used that as an unused variable name, and then the compiler generates a warning of unused variable. Lol.

Copy link
Contributor Author

@ro-i ro-i Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current behavior does seem a little inconsistent: https://godbolt.org/z/eM1xj54P7
Interestingly, a standalone variable with the name _ is not flagged as unused

// 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"
" return OperandMap[InstrIdx][(unsigned)Name];\n"
"}\n";
}

static void
emitGetOperandIdxName(raw_ostream &OS,
MapVector<StringRef, unsigned> OperandNameToID,
const MapVector<SmallVector<int>, unsigned> &OperandMap,
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;
}
OS << " static constexpr OpName OperandMap[][" << MaxNumOperands
<< "] = {\n";
for (const auto &[OpList, _] : OperandMap) {
SmallVector<unsigned> 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) {
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"
" return OperandMap[InstrIdx][(unsigned)Idx];\n"
"}\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.
Expand All @@ -242,11 +329,6 @@ void InstrInfoEmitter::emitOperandNameMappings(
ArrayRef<const CodeGenInstruction *> 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<StringRef, unsigned> OperandNameToID;

Expand Down Expand Up @@ -285,53 +367,38 @@ 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 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";

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, OperandMap, MaxOperandNo, NumOperandNames);
emitGetOperandIdxName(OS, OperandNameToID, OperandMap, MaxNumOperands,
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";
}
Expand Down
Loading