[TableGen][DecoderEmitter] Store HW mode ID instead of name (NFC)

[s-barannikov]

This simplifies code a bit.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

This simplifies code a bit.

---

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

1 Files Affected:

• (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+28-40)

diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index f69e47e49c1eb..271a011722e62 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -208,14 +208,14 @@ struct DecoderTableInfo {
 struct EncodingAndInst {
   const Record *EncodingDef;
   const CodeGenInstruction *Inst;
-  StringRef HwModeName;
+  unsigned HwModeID;
 
   EncodingAndInst(const Record *EncodingDef, const CodeGenInstruction *Inst,
-                  StringRef HwModeName = "")
-      : EncodingDef(EncodingDef), Inst(Inst), HwModeName(HwModeName) {}
+                  unsigned HwModeID = DefaultMode)
+      : EncodingDef(EncodingDef), Inst(Inst), HwModeID(HwModeID) {}
 };
 
-using NamespacesHwModesMap = std::map<std::string, std::set<StringRef>>;
+using NamespacesHwModesMap = std::map<std::string, std::set<unsigned>>;
 
 class DecoderEmitter {
   const RecordKeeper &RK;
@@ -2396,7 +2396,7 @@ static bool Check(DecodeStatus &Out, DecodeStatus In) {
 // Collect all HwModes referenced by the target for encoding purposes,
 // returning a vector of corresponding names.
 static void collectHwModesReferencedForEncodings(
-    const CodeGenHwModes &HWM, std::vector<StringRef> &Names,
+    const CodeGenHwModes &HWM, std::vector<unsigned> &HwModeIDs,
     NamespacesHwModesMap &NamespacesWithHwModes) {
   SmallBitVector BV(HWM.getNumModeIds());
   for (const auto &MS : HWM.getHwModeSelects()) {
@@ -2404,34 +2404,25 @@ static void collectHwModesReferencedForEncodings(
       if (EncodingDef->isSubClassOf("InstructionEncoding")) {
         std::string DecoderNamespace =
             EncodingDef->getValueAsString("DecoderNamespace").str();
-        if (HwModeID == DefaultMode) {
-          NamespacesWithHwModes[DecoderNamespace].insert("");
-        } else {
-          NamespacesWithHwModes[DecoderNamespace].insert(
-              HWM.getMode(HwModeID).Name);
-        }
+        NamespacesWithHwModes[DecoderNamespace].insert(HwModeID);
         BV.set(HwModeID);
       }
     }
   }
-  transform(BV.set_bits(), std::back_inserter(Names), [&HWM](const int &M) {
-    if (M == DefaultMode)
-      return StringRef("");
-    return HWM.getModeName(M, /*IncludeDefault=*/true);
-  });
+  append_range(HwModeIDs, BV.set_bits());
 }
 
 static void
 handleHwModesUnrelatedEncodings(const CodeGenInstruction *Instr,
-                                ArrayRef<StringRef> HwModeNames,
+                                ArrayRef<unsigned> HwModeIDs,
                                 NamespacesHwModesMap &NamespacesWithHwModes,
                                 std::vector<EncodingAndInst> &GlobalEncodings) {
   const Record *InstDef = Instr->TheDef;
 
   switch (DecoderEmitterSuppressDuplicates) {
   case SUPPRESSION_DISABLE: {
-    for (StringRef HwModeName : HwModeNames)
-      GlobalEncodings.emplace_back(InstDef, Instr, HwModeName);
+    for (unsigned HwModeID : HwModeIDs)
+      GlobalEncodings.emplace_back(InstDef, Instr, HwModeID);
     break;
   }
   case SUPPRESSION_LEVEL1: {
@@ -2439,17 +2430,17 @@ handleHwModesUnrelatedEncodings(const CodeGenInstruction *Instr,
         InstDef->getValueAsString("DecoderNamespace").str();
     auto It = NamespacesWithHwModes.find(DecoderNamespace);
     if (It != NamespacesWithHwModes.end()) {
-      for (StringRef HwModeName : It->second)
-        GlobalEncodings.emplace_back(InstDef, Instr, HwModeName);
+      for (unsigned HwModeID : It->second)
+        GlobalEncodings.emplace_back(InstDef, Instr, HwModeID);
     } else {
       // Only emit the encoding once, as it's DecoderNamespace doesn't
       // contain any HwModes.
-      GlobalEncodings.emplace_back(InstDef, Instr, "");
+      GlobalEncodings.emplace_back(InstDef, Instr, DefaultMode);
     }
     break;
   }
   case SUPPRESSION_LEVEL2:
-    GlobalEncodings.emplace_back(InstDef, Instr, "");
+    GlobalEncodings.emplace_back(InstDef, Instr, DefaultMode);
     break;
   }
 }
@@ -2480,13 +2471,13 @@ namespace {
 
   // First, collect all encoding-related HwModes referenced by the target.
   // And establish a mapping table between DecoderNamespace and HwMode.
-  // If HwModeNames is empty, add the empty string so we always have one HwMode.
+  // If HwModeNames is empty, add the default mode so we always have one HwMode.
   const CodeGenHwModes &HWM = Target.getHwModes();
-  std::vector<StringRef> HwModeNames;
+  std::vector<unsigned> HwModeIDs;
   NamespacesHwModesMap NamespacesWithHwModes;
-  collectHwModesReferencedForEncodings(HWM, HwModeNames, NamespacesWithHwModes);
-  if (HwModeNames.empty())
-    HwModeNames.push_back("");
+  collectHwModesReferencedForEncodings(HWM, HwModeIDs, NamespacesWithHwModes);
+  if (HwModeIDs.empty())
+    HwModeIDs.push_back(DefaultMode);
 
   const auto &NumberedInstructions = Target.getInstructions();
   NumberedEncodings.reserve(NumberedInstructions.size());
@@ -2494,20 +2485,14 @@ namespace {
     const Record *InstDef = NumberedInstruction->TheDef;
     if (const Record *RV = InstDef->getValueAsOptionalDef("EncodingInfos")) {
       EncodingInfoByHwMode EBM(RV, HWM);
-      for (auto [HwModeID, EncodingDef] : EBM) {
-        // DecoderTables with DefaultMode should not have any suffix.
-        if (HwModeID == DefaultMode) {
-          NumberedEncodings.emplace_back(EncodingDef, NumberedInstruction, "");
-        } else {
-          NumberedEncodings.emplace_back(EncodingDef, NumberedInstruction,
-                                         HWM.getMode(HwModeID).Name);
-        }
-      }
+      for (auto [HwModeID, EncodingDef] : EBM)
+        NumberedEncodings.emplace_back(EncodingDef, NumberedInstruction,
+                                       HwModeID);
       continue;
     }
     // This instruction is encoded the same on all HwModes.
     // According to user needs, provide varying degrees of suppression.
-    handleHwModesUnrelatedEncodings(NumberedInstruction, HwModeNames,
+    handleHwModesUnrelatedEncodings(NumberedInstruction, HwModeIDs,
                                     NamespacesWithHwModes, NumberedEncodings);
   }
   for (const Record *NumberedAlias :
@@ -2554,8 +2539,11 @@ namespace {
       }
       std::string DecoderNamespace =
           EncodingDef->getValueAsString("DecoderNamespace").str();
-      if (!NumberedEncoding.HwModeName.empty())
-        DecoderNamespace += "_" + NumberedEncoding.HwModeName.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);
     } else {
       NumEncodingsOmitted++;

[rastogishubham]

Hi this change has broken greendragon:

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/31586/ and
https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA-as/629/

20:34:43  In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/utils/TableGen/DecoderEmitter.cpp:14:
20:34:43  In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/utils/TableGen/Common/CodeGenHwModes.h:14:
20:34:43  In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/include/llvm/ADT/DenseMap.h:20:
20:34:43  In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/include/llvm/ADT/STLExtras.h:21:
20:34:43  In file included from /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/include/llvm/ADT/Hashing.h:53:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/algorithm:1913:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/chrono:746:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__chrono/convert_to_tm.h:19:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__chrono/statically_widen.h:17:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__format/concepts.h:17:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__format/format_parse_context.h:15:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/string_view:1027:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/functional:515:
20:34:43  In file included from /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/__functional/boyer_moore_searcher.h:26:
20:34:43  /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/c++/v1/vector:1376:19: error: object of type 'llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>' cannot be assigned because its copy assignment operator is implicitly deleted
20:34:43              __mid =  __first;
20:34:43                    ^
20:34:43  /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/utils/TableGen/DecoderEmitter.cpp:2404:13: note: in instantiation of function template specialization 'std::vector<unsigned int>::assign<llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>, 0>' requested here
20:34:43    HwModeIDs.assign(BV.set_bits_begin(), BV.set_bits_end());
20:34:43              ^
20:34:43  /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/llvm/include/llvm/ADT/BitVector.h:35:21: note: copy assignment operator of 'const_set_bits_iterator_impl<llvm::SmallBitVector>' is implicitly deleted because field 'Parent' is of reference type 'const llvm::SmallBitVector &'
20:34:43    const BitVectorT &Parent;
20:34:43                      ^

It has been reverted with 5612dc5 to make sure greendragon stays green

[s-barannikov]

@rastogishubham
Hi, thanks for the report. I've created #154212 with a fix.