Skip to content

Conversation

@s-barannikov
Copy link
Contributor

Previously, HW mode name was appended to decoder namespace name when enumerating encodings, and then emitTable appended the bit width to it to form the final table name. Let's do this all in one place.
A nice side effect is that this allows us to avoid having to deal with std::string.

The changes in the tests are caused by the different order of tables.

Previously, HW mode name was appended to decoder namespace name when
enumerating encodings, and then emitTable appended the bit width to it
to form the final table name. Let's do this all in one place.
A nice side effect is that this allows us to avoid having to deal with
std::string.

The changes in the tests are caused by the different order of tables.
@llvmbot
Copy link
Member

llvmbot commented Aug 19, 2025

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

Previously, HW mode name was appended to decoder namespace name when enumerating encodings, and then emitTable appended the bit width to it to form the final table name. Let's do this all in one place.
A nice side effect is that this allows us to avoid having to deal with std::string.

The changes in the tests are caused by the different order of tables.


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

3 Files Affected:

  • (modified) llvm/test/TableGen/HwModeEncodeDecode2.td (+6-6)
  • (modified) llvm/test/TableGen/HwModeEncodeDecode3.td (+12-12)
  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+21-23)
diff --git a/llvm/test/TableGen/HwModeEncodeDecode2.td b/llvm/test/TableGen/HwModeEncodeDecode2.td
index cf96dda6c8bf3b..dc8773699a4666 100644
--- a/llvm/test/TableGen/HwModeEncodeDecode2.td
+++ b/llvm/test/TableGen/HwModeEncodeDecode2.td
@@ -93,10 +93,6 @@ let OutOperandList = (outs) in {
   }
 }
 
-// DECODER-LABEL: DecoderTableAlt_ModeA32[] =
-// DECODER-DAG: Opcode: unrelated
-// DECODER-LABEL: DecoderTableAlt_ModeB32[] =
-// DECODER-DAG: Opcode: unrelated
 // DECODER-LABEL: DecoderTable_ModeA32[] =
 // DECODER-DAG: Opcode: fooTypeEncA:foo
 // DECODER-DAG: Opcode: bar
@@ -104,11 +100,13 @@ let OutOperandList = (outs) in {
 // DECODER-DAG: Opcode: fooTypeEncB:foo
 // DECODER-DAG: Opcode: fooTypeEncA:baz
 // DECODER-DAG: Opcode: bar
+// DECODER-LABEL: DecoderTableAlt_ModeA32[] =
+// DECODER-DAG: Opcode: unrelated
+// DECODER-LABEL: DecoderTableAlt_ModeB32[] =
+// DECODER-DAG: Opcode: unrelated
 
 // DECODER-SUPPRESS-LABEL: DecoderTable32[] =
 // DECODER-SUPPRESS-DAG: Opcode: bar
-// DECODER-SUPPRESS-LABEL: DecoderTableAlt32[] =
-// DECODER-SUPPRESS-DAG: Opcode: unrelated
 // DECODER-SUPPRESS-LABEL: DecoderTable_ModeA32[] =
 // DECODER-SUPPRESS-DAG: Opcode: fooTypeEncA:foo
 // DECODER-SUPPRESS-NOT: Opcode: bar
@@ -116,3 +114,5 @@ let OutOperandList = (outs) in {
 // DECODER-SUPPRESS-DAG: Opcode: fooTypeEncB:foo
 // DECODER-SUPPRESS-DAG: Opcode: fooTypeEncA:baz
 // DECODER-SUPPRESS-NOT: Opcode: bar
+// DECODER-SUPPRESS-LABEL: DecoderTableAlt32[] =
+// DECODER-SUPPRESS-DAG: Opcode: unrelated
diff --git a/llvm/test/TableGen/HwModeEncodeDecode3.td b/llvm/test/TableGen/HwModeEncodeDecode3.td
index c4d488d9d5f8f5..2629af26be7c0f 100644
--- a/llvm/test/TableGen/HwModeEncodeDecode3.td
+++ b/llvm/test/TableGen/HwModeEncodeDecode3.td
@@ -120,14 +120,6 @@ def unrelated: Instruction {
 // DECODER-DAG: Opcode: bar
 // DECODER-LABEL: DecoderTable64[] =
 // DECODER-DAG: Opcode: fooTypeEncDefault:foo
-// DECODER-LABEL: DecoderTableAlt32[] =
-// DECODER-DAG: Opcode: unrelated
-// DECODER-LABEL: DecoderTableAlt_ModeA32[] =
-// DECODER-DAG: Opcode: unrelated
-// DECODER-LABEL: DecoderTableAlt_ModeB32[] =
-// DECODER-DAG: Opcode: unrelated
-// DECODER-LABEL: DecoderTableAlt_ModeC32[] =
-// DECODER-DAG: Opcode: unrelated
 // DECODER-LABEL: DecoderTable_ModeA32[] =
 // DECODER-DAG: Opcode: fooTypeEncA:foo
 // DECODER-DAG: Opcode: bar
@@ -138,6 +130,14 @@ def unrelated: Instruction {
 // DECODER-LABEL: DecoderTable_ModeC32[] =
 // DECODER-DAG: Opcode: fooTypeEncC:foo
 // DECODER-DAG: Opcode: bar
+// DECODER-LABEL: DecoderTableAlt32[] =
+// DECODER-DAG: Opcode: unrelated
+// DECODER-LABEL: DecoderTableAlt_ModeA32[] =
+// DECODER-DAG: Opcode: unrelated
+// DECODER-LABEL: DecoderTableAlt_ModeB32[] =
+// DECODER-DAG: Opcode: unrelated
+// DECODER-LABEL: DecoderTableAlt_ModeC32[] =
+// DECODER-DAG: Opcode: unrelated
 
 // Under the 'O1' optimization level, unnecessary duplicate tables will be eliminated,
 // reducing the four ‘Alt’ tables down to just one.
@@ -145,8 +145,6 @@ def unrelated: Instruction {
 // DECODER-SUPPRESS-O1-DAG: Opcode: bar
 // DECODER-SUPPRESS-O1-LABEL: DecoderTable64[] =
 // DECODER-SUPPRESS-O1-DAG: Opcode: fooTypeEncDefault:foo
-// DECODER-SUPPRESS-O1-LABEL: DecoderTableAlt32[] =
-// DECODER-SUPPRESS-O1-DAG: Opcode: unrelated
 // DECODER-SUPPRESS-O1-LABEL: DecoderTable_ModeA32[] =
 // DECODER-SUPPRESS-O1-DAG: Opcode: fooTypeEncA:foo
 // DECODER-SUPPRESS-O1-DAG: Opcode: bar
@@ -157,6 +155,8 @@ def unrelated: Instruction {
 // DECODER-SUPPRESS-O1-LABEL: DecoderTable_ModeC32[] =
 // DECODER-SUPPRESS-O1-DAG: Opcode: fooTypeEncC:foo
 // DECODER-SUPPRESS-O1-DAG: Opcode: bar
+// DECODER-SUPPRESS-O1-LABEL: DecoderTableAlt32[] =
+// DECODER-SUPPRESS-O1-DAG: Opcode: unrelated
 
 // Under the 'O2' optimization condition, instructions possessing the 'EncodingByHwMode'
 // attribute will be extracted from their original DecoderNamespace and placed into their
@@ -169,8 +169,6 @@ def unrelated: Instruction {
 // DECODER-SUPPRESS-O2-LABEL: DecoderTable64[] =
 // DECODER-SUPPRESS-O2-NOT: Opcode: bar
 // DECODER-SUPPRESS-O2-DAG: Opcode: fooTypeEncDefault:foo
-// DECODER-SUPPRESS-O2-LABEL: DecoderTableAlt32[] =
-// DECODER-SUPPRESS-O2-DAG: Opcode: unrelated
 // DECODER-SUPPRESS-O2-LABEL: DecoderTable_ModeA32[] =
 // DECODER-SUPPRESS-O2-DAG: Opcode: fooTypeEncA:foo
 // DECODER-SUPPRESS-O2-NOT: Opcode: bar
@@ -181,6 +179,8 @@ def unrelated: Instruction {
 // DECODER-SUPPRESS-O2-LABEL: DecoderTable_ModeC32[] =
 // DECODER-SUPPRESS-O2-DAG: Opcode: fooTypeEncC:foo
 // DECODER-SUPPRESS-O2-NOT: Opcode: bar
+// DECODER-SUPPRESS-O2-LABEL: DecoderTableAlt32[] =
+// DECODER-SUPPRESS-O2-DAG: Opcode: unrelated
 
 // For 'bar' and 'unrelated', we didn't assign any HwModes for them,
 // they should keep the same in the following four tables.
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index c1afbbdbcf4591..c16d5b5d317d6b 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -230,7 +230,7 @@ class DecoderEmitter {
   // Emit the decoder state machine table. Returns a mask of MCD decoder ops
   // that were emitted.
   unsigned emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
-                     unsigned BitWidth, StringRef Namespace,
+                     StringRef Namespace, unsigned HwModeID, unsigned BitWidth,
                      ArrayRef<unsigned> EncodingIDs) const;
   void emitInstrLenTable(formatted_raw_ostream &OS,
                          ArrayRef<unsigned> InstrLen) const;
@@ -805,8 +805,8 @@ unsigned Filter::usefulness() const {
 // Emit the decoder state machine table. Returns a mask of MCD decoder ops
 // that were emitted.
 unsigned DecoderEmitter::emitTable(formatted_raw_ostream &OS,
-                                   DecoderTable &Table, unsigned BitWidth,
-                                   StringRef Namespace,
+                                   DecoderTable &Table, StringRef Namespace,
+                                   unsigned HwModeID, unsigned BitWidth,
                                    ArrayRef<unsigned> EncodingIDs) const {
   // We'll need to be able to map from a decoded opcode into the corresponding
   // EncodingID for this specific combination of BitWidth and Namespace. This
@@ -818,8 +818,10 @@ unsigned DecoderEmitter::emitTable(formatted_raw_ostream &OS,
     OpcodeToEncodingID[Target.getInstrIntValue(InstDef)] = EncodingID;
   }
 
-  OS << "static const uint8_t DecoderTable" << Namespace << BitWidth
-     << "[] = {\n";
+  OS << "static const uint8_t DecoderTable" << Namespace;
+  if (HwModeID != DefaultMode)
+    OS << '_' << Target.getHwModes().getModeName(HwModeID);
+  OS << BitWidth << "[] = {\n";
 
   // Emit ULEB128 encoded value to OS, returning the number of bytes emitted.
   auto emitULEB128 = [](DecoderTable::const_iterator &I,
@@ -2501,8 +2503,9 @@ namespace {
         NumberedAlias,
         &Target.getInstruction(NumberedAlias->getValueAsDef("AliasOf")));
 
-  // Map of (namespace, size) tuple to encoding IDs.
-  std::map<std::pair<std::string, unsigned>, std::vector<unsigned>> EncMap;
+  // Map of (namespace, hwmode, size) tuple to encoding IDs.
+  std::map<std::tuple<StringRef, unsigned, unsigned>, std::vector<unsigned>>
+      EncMap;
   std::map<unsigned, std::vector<OperandInfo>> Operands;
   std::vector<unsigned> InstrLen;
   bool IsVarLenInst = Target.hasVariableLengthEncodings();
@@ -2537,14 +2540,10 @@ namespace {
         MaxInstLen = std::max(MaxInstLen, Len);
         InstrLen[NEI] = Len;
       }
-      std::string DecoderNamespace =
-          EncodingDef->getValueAsString("DecoderNamespace").str();
-      // DecoderTables with DefaultMode should not have any suffix.
-      if (NumberedEncoding.HwModeID != DefaultMode) {
-        StringRef HwModeName = HWM.getModeName(NumberedEncoding.HwModeID);
-        DecoderNamespace += ("_" + HwModeName).str();
-      }
-      EncMap[{DecoderNamespace, Size}].push_back(NEI);
+      StringRef DecoderNamespace =
+          EncodingDef->getValueAsString("DecoderNamespace");
+      EncMap[{DecoderNamespace, NumberedEncoding.HwModeID, Size}].push_back(
+          NEI);
     } else {
       NumEncodingsOmitted++;
     }
@@ -2552,12 +2551,11 @@ namespace {
 
   DecoderTableInfo TableInfo;
   unsigned OpcodeMask = 0;
-  for (const auto &[NSAndByteSize, EncodingIDs] : EncMap) {
-    const std::string &DecoderNamespace = NSAndByteSize.first;
-    const unsigned BitWidth = 8 * NSAndByteSize.second;
-    // Emit the decoder for this namespace+width combination.
-    FilterChooser FC(NumberedEncodings, EncodingIDs, Operands,
-                     IsVarLenInst ? MaxInstLen : BitWidth, this);
+  for (const auto &[Key, EncodingIDs] : EncMap) {
+    auto [DecoderNamespace, HwModeID, Size] = Key;
+    const unsigned BitWidth = IsVarLenInst ? MaxInstLen : 8 * Size;
+    // Emit the decoder for this (namespace, hwmode, width) combination.
+    FilterChooser FC(NumberedEncodings, EncodingIDs, Operands, BitWidth, this);
 
     // The decode table is cleared for each top level decoder function. The
     // predicates and decoders themselves, however, are shared across all
@@ -2573,8 +2571,8 @@ namespace {
     TableInfo.Table.push_back(MCD::OPC_Fail);
 
     // Print the table to the output stream.
-    OpcodeMask |= emitTable(OS, TableInfo.Table, FC.getBitWidth(),
-                            DecoderNamespace, EncodingIDs);
+    OpcodeMask |= emitTable(OS, TableInfo.Table, DecoderNamespace, HwModeID,
+                            BitWidth, EncodingIDs);
   }
 
   // For variable instruction, we emit a instruction length table

@s-barannikov s-barannikov enabled auto-merge (squash) August 19, 2025 02:32
@s-barannikov s-barannikov disabled auto-merge August 19, 2025 03:00
@s-barannikov s-barannikov merged commit c8c2218 into llvm:main Aug 19, 2025
11 checks passed
@s-barannikov s-barannikov deleted the tablegen/decoder/table-name branch August 19, 2025 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants