[TableGen] Fix OperandMap table size

[mariusz-sikora-at-amd]

• add missing value for OPERAND_LAST type. If function 'getNamedOperandIdx' is called with NamedIdx=OPERAND_LAST then we are accessing first element of next row in 2dim table.
• in most cases this will work unnoticed because 2dim OperandMap is sparse with most elements set to '-1'.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Mariusz Sikora (mariusz-sikora-at-amd)

Changes

• add missing value for OPERAND_LAST type. If function 'getNamedOperandIdx' is called with NamedIdx=OPERAND_LAST then we are accessing first element of next row in 2dim table.
• in most cases this will work unnoticed because 2dim OperandMap is sparse with most elements set to '-1'.

---

Full diff: https://github.com/llvm/llvm-project/pull/125424.diff

1 Files Affected:

• (modified) llvm/utils/TableGen/InstrInfoEmitter.cpp (+4-3)

diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index 97c00ad4924197..ecf30c95f4b76b 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -300,8 +300,8 @@ void InstrInfoEmitter::emitOperandNameMappings(
     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";
+    OS << "  static constexpr " << Type << " OperandMap[]["
+       << NumOperandNames + 1 << "] = {\n";
     for (const auto &Entry : OperandMap) {
       const std::map<unsigned, unsigned> &OpList = Entry.first;
 
@@ -311,7 +311,8 @@ void InstrInfoEmitter::emitOperandNameMappings(
         auto Iter = OpList.find(ID);
         OS << (Iter != OpList.end() ? (int)Iter->second : -1) << ", ";
       }
-      OS << "},\n";
+      // value for OPERAND_LAST
+      OS << "-1 },\n";
     }
     OS << "  };\n";
 

[jurahul]

It may be better to deal with this as a separate if in the generated code instead of adding one more row to the table.

[jurahul]

Actually, I'd say this should be an assert? We should not be passing OPERAND_LAST into this function ever

[mariusz-sikora-at-amd]

OS << "assert(NamesIdx < OPERAND_LAST);"; ?

[jurahul]

Yeah that will work. Though I am changing the code here in #125313 If you want I can incorporate that change in the PR (since both names are changing in that PR)

…

On Sun, Feb 2, 2025 at 12:10 PM Mariusz Sikora ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In llvm/utils/TableGen/InstrInfoEmitter.cpp <#125424 (comment)>: > @@ -300,8 +300,8 @@ void InstrInfoEmitter::emitOperandNameMappings( assert(MaxOperandNo <= INT16_MAX && OS << "assert(NamesIdx < OPERAND_LAST);"; ? — Reply to this email directly, view it on GitHub <#125424 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APRMUB6GVUYTMJT5UF6ITPD2NZ3SHAVCNFSM6AAAAABWKTEW26VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBYGYZDOOBZGM> . You are receiving this because your review was requested.Message ID: ***@***.***>

[mariusz-sikora-at-amd]

Yeah that will work. Though I am changing the code here in #125313 If you want I can incorporate that change in the PR (since both names are changing in that PR)

Please do. Thanks. I will close this PR.

[jurahul]

Will do, thanks!