Skip to content

Conversation

@s-barannikov
Copy link
Contributor

This reverts commit 5612dc5.

Reland with MacOS build fixed.

@llvmbot
Copy link
Member

llvmbot commented Aug 18, 2025

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

This reverts commit 5612dc5.

Reland with MacOS build fixed.


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

1 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+32-42)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 496c5390625aa..34fb5f4dddc13 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;
@@ -2391,10 +2391,9 @@ static bool Check(DecodeStatus &Out, DecodeStatus In) {
 )";
 }
 
-// Collect all HwModes referenced by the target for encoding purposes,
-// returning a vector of corresponding names.
+// Collect all HwModes referenced by the target for encoding purposes.
 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()) {
@@ -2402,34 +2401,28 @@ 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);
-  });
+  // FIXME: Can't do `HwModeIDs.assign(BV.set_bits_begin(), BV.set_bits_end())`
+  //   because const_set_bits_iterator_impl is not copy-assignable.
+  //   This breaks some MacOS builds.
+  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: {
@@ -2437,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;
   }
 }
@@ -2478,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());
@@ -2492,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 :
@@ -2552,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++;

@s-barannikov
Copy link
Contributor Author

s-barannikov commented Aug 18, 2025

@rastogishubham
I've set it to auto-merge. Please let me know if it breaks again and I'll quickly fix it (I have a more reliable candidate fix in mind).

@s-barannikov s-barannikov enabled auto-merge (squash) August 18, 2025 22:19
@s-barannikov s-barannikov merged commit 0cd4ae9 into llvm:main Aug 18, 2025
11 checks passed
@s-barannikov s-barannikov deleted the reland-154052 branch August 18, 2025 22:31
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 18, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building llvm at step 5 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/25626

Here is the relevant piece of the build log for the reference
Step 5 (build-unified-tree) failure: build (failure)
...
4.508 [4918/8/1276] Building arm_neon.inc...
4.519 [4917/8/1277] Building arm_mve_builtin_sema.inc...
4.519 [4916/8/1278] Building arm_mve_builtins.inc...
4.526 [4915/8/1279] Building arm_mve_builtin_aliases.inc...
4.533 [4914/8/1280] Building arm_mve_builtin_cg.inc...
4.535 [4913/8/1281] Building arm_sve_typeflags.inc...
4.551 [4912/8/1282] Building arm_sve_streaming_attrs.inc...
4.555 [4911/8/1283] Building arm_sme_builtin_cg.inc...
4.556 [4910/8/1284] Building arm_sme_builtins.inc...
4.560 [4909/8/1285] Building CXX object utils/TableGen/CMakeFiles/llvm-tblgen.dir/DecoderEmitter.cpp.o
FAILED: [code=1] utils/TableGen/CMakeFiles/llvm-tblgen.dir/DecoderEmitter.cpp.o 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes CCACHE_SLOPPINESS=pch_defines,time_macros /opt/homebrew/bin/ccache /usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/Users/buildbot/buildbot-root/aarch64-darwin/build/utils/TableGen -I/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/TableGen -I/Users/buildbot/buildbot-root/aarch64-darwin/build/include -I/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/include -isystem /opt/homebrew/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -O3 -DNDEBUG -std=c++17 -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT utils/TableGen/CMakeFiles/llvm-tblgen.dir/DecoderEmitter.cpp.o -MF utils/TableGen/CMakeFiles/llvm-tblgen.dir/DecoderEmitter.cpp.o.d -o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DecoderEmitter.cpp.o -c /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/TableGen/DecoderEmitter.cpp
In file included from /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/TableGen/DecoderEmitter.cpp:14:
In file included from /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/TableGen/Common/CodeGenHwModes.h:14:
In file included from /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/include/llvm/ADT/DenseMap.h:20:
In file included from /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/include/llvm/ADT/STLExtras.h:33:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/functional:526:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/__functional/boyer_moore_searcher.h:27:
/Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/vector:1951:21: error: object of type 'llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>' cannot be assigned because its copy assignment operator is implicitly deleted
                __m = __first;
                    ^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/vector:1930:10: note: in instantiation of function template specialization 'std::vector<unsigned int>::__insert_with_size<llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>, llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>>' requested here
  return __insert_with_size(__position, __first, __last, std::distance(__first, __last));
         ^
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/include/llvm/ADT/STLExtras.h:2156:5: note: in instantiation of function template specialization 'std::vector<unsigned int>::insert<llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>, 0>' requested here
  C.insert(C.end(), adl_begin(R), adl_end(R));
    ^
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/TableGen/DecoderEmitter.cpp:2412:3: note: in instantiation of function template specialization 'llvm::append_range<std::vector<unsigned int>, llvm::iterator_range<llvm::const_set_bits_iterator_impl<llvm::SmallBitVector>>>' requested here
  append_range(HwModeIDs, BV.set_bits());
  ^
/Users/buildbot/buildbot-root/aarch64-darwin/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 &'
  const BitVectorT &Parent;
                    ^
1 error generated.
4.569 [4909/7/1286] Building arm_sme_builtins_za_state.inc...
4.570 [4909/6/1287] Building arm_sme_streaming_attrs.inc...
4.572 [4909/5/1288] Building arm_sme_sema_rangechecks.inc...
4.583 [4909/4/1289] Building arm_sve_builtin_cg.inc...
4.593 [4909/3/1290] Building arm_sve_builtins.inc...
4.595 [4909/2/1291] Building arm_sve_sema_rangechecks.inc...
4.906 [4909/1/1292] Building CXX object lib/MC/MCParser/CMakeFiles/LLVMMCParser.dir/AsmParser.cpp.o
ninja: build stopped: subcommand failed.

@s-barannikov
Copy link
Contributor Author

Looks like the fix didn't help (this time I got an e-mail notification). I'll push another one shortly.

@s-barannikov
Copy link
Contributor Author

Looks like the fix didn't help (this time I got an e-mail notification). I'll push another one shortly.

61a859b

@dyung
Copy link
Collaborator

dyung commented Aug 18, 2025

Looks like the fix didn't help (this time I got an e-mail notification). I'll push another one shortly.

I don't believe Green Dragon sends email on failures. There are 2 MacOS bots in the LLVM Buildbot that I setup that when they fail should cause emails and notifications to be sent out when they fail (like you received this time)

@s-barannikov
Copy link
Contributor Author

s-barannikov commented Aug 18, 2025

@dyung Thanks for the info. Do you happen to know why MacOS pre-merge checks are disabled? E.g.:

image

I see that on every opened PR.

@dyung
Copy link
Collaborator

dyung commented Aug 18, 2025

@dyung Thanks for the info. Do you happen to know why MacOS pre-merge checks are disabled? E.g.:

image I see that on every opened PR.

Sorry I do not. I only run the two post-submit bots on MacOS, I am not involved with the pre-commit stuff.

@s-barannikov
Copy link
Contributor Author

I see, thanks!

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.

4 participants