Skip to content

Revert "LLVMContext: Cleanup registration of known bundle IDs" #120396

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.

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

Conversation

jplehr
Copy link
Contributor

@jplehr jplehr commented Dec 18, 2024

Reverts #120359

This broke buildbots with older GCC versions: https://lab.llvm.org/buildbot/#/builders/140/builds/13302

Failure

7.815 [4326/32/2685] Building CXX object lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o
FAILED: lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o 
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/IR -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR -Iinclude -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++1z -MD -MT lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o -MF lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o.d -o lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o -c /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/Hashing.h:49:0,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/ArrayRef.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/ConstantsContext.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContextImpl.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp:15:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp: In function ‘constexpr llvm::StringRef knownBundleName(unsigned int)’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/Support/ErrorHandling.h:144:36: error: call to non-constexpr function ‘void llvm::llvm_unreachable_internal(const char*, const char*, unsigned int)’
   ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp:60:3: note: in expansion of macro ‘llvm_unreachable’
   llvm_unreachable("covered switch");
   ^~~~~~~~~~~~~~~~

@jplehr jplehr requested a review from arsenm December 18, 2024 10:01
@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2024

@llvm/pr-subscribers-llvm-ir

Author: Jan Patrick Lehr (jplehr)

Changes

Reverts llvm/llvm-project#120359

This broke buildbots with older GCC versions: https://lab.llvm.org/buildbot/#/builders/140/builds/13302

Failure

7.815 [4326/32/2685] Building CXX object lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o
FAILED: lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o 
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/IR -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR -Iinclude -I/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++1z -MD -MT lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o -MF lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o.d -o lib/IR/CMakeFiles/LLVMCore.dir/LLVMContext.cpp.o -c /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp
In file included from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/Hashing.h:49:0,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/ADT/ArrayRef.h:12,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/ConstantsContext.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContextImpl.h:17,
                 from /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp:15:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp: In function ‘constexpr llvm::StringRef knownBundleName(unsigned int)’:
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/include/llvm/Support/ErrorHandling.h:144:36: error: call to non-constexpr function ‘void llvm::llvm_unreachable_internal(const char*, const char*, unsigned int)’
   ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/lib/IR/LLVMContext.cpp:60:3: note: in expansion of macro ‘llvm_unreachable’
   llvm_unreachable("covered switch");
   ^~~~~~~~~~~~~~~~

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

1 Files Affected:

  • (modified) llvm/lib/IR/LLVMContext.cpp (+50-35)
diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp
index 9acc15f11316a2..eb51a751bfa088 100644
--- a/llvm/lib/IR/LLVMContext.cpp
+++ b/llvm/lib/IR/LLVMContext.cpp
@@ -31,35 +31,6 @@
 
 using namespace llvm;
 
-static constexpr StringRef knownBundleName(unsigned BundleTagID) {
-  switch (BundleTagID) {
-  case LLVMContext::OB_deopt:
-    return "deopt";
-  case LLVMContext::OB_funclet:
-    return "funclet";
-  case LLVMContext::OB_gc_transition:
-    return "gc-transition";
-  case LLVMContext::OB_cfguardtarget:
-    return "cfguardtarget";
-  case LLVMContext::OB_preallocated:
-    return "preallocated";
-  case LLVMContext::OB_gc_live:
-    return "gc-live";
-  case LLVMContext::OB_clang_arc_attachedcall:
-    return "clang.arc.attachedcall";
-  case LLVMContext::OB_ptrauth:
-    return "ptrauth";
-  case LLVMContext::OB_kcfi:
-    return "kcfi";
-  case LLVMContext::OB_convergencectrl:
-    return "convergencectrl";
-  default:
-    llvm_unreachable("unknown bundle id");
-  }
-
-  llvm_unreachable("covered switch");
-}
-
 LLVMContext::LLVMContext() : pImpl(new LLVMContextImpl(*this)) {
   // Create the fixed metadata kinds. This is done in the same order as the
   // MD_* enum values so that they correspond.
@@ -75,12 +46,56 @@ LLVMContext::LLVMContext() : pImpl(new LLVMContextImpl(*this)) {
     (void)ID;
   }
 
-  for (unsigned BundleTagID = LLVMContext::OB_deopt;
-       BundleTagID <= LLVMContext::OB_convergencectrl; ++BundleTagID) {
-    [[maybe_unused]] const auto *Entry =
-        pImpl->getOrInsertBundleTag(knownBundleName(BundleTagID));
-    assert(Entry->second == BundleTagID && "operand bundle id drifted!");
-  }
+  auto *DeoptEntry = pImpl->getOrInsertBundleTag("deopt");
+  assert(DeoptEntry->second == LLVMContext::OB_deopt &&
+         "deopt operand bundle id drifted!");
+  (void)DeoptEntry;
+
+  auto *FuncletEntry = pImpl->getOrInsertBundleTag("funclet");
+  assert(FuncletEntry->second == LLVMContext::OB_funclet &&
+         "funclet operand bundle id drifted!");
+  (void)FuncletEntry;
+
+  auto *GCTransitionEntry = pImpl->getOrInsertBundleTag("gc-transition");
+  assert(GCTransitionEntry->second == LLVMContext::OB_gc_transition &&
+         "gc-transition operand bundle id drifted!");
+  (void)GCTransitionEntry;
+
+  auto *CFGuardTargetEntry = pImpl->getOrInsertBundleTag("cfguardtarget");
+  assert(CFGuardTargetEntry->second == LLVMContext::OB_cfguardtarget &&
+         "cfguardtarget operand bundle id drifted!");
+  (void)CFGuardTargetEntry;
+
+  auto *PreallocatedEntry = pImpl->getOrInsertBundleTag("preallocated");
+  assert(PreallocatedEntry->second == LLVMContext::OB_preallocated &&
+         "preallocated operand bundle id drifted!");
+  (void)PreallocatedEntry;
+
+  auto *GCLiveEntry = pImpl->getOrInsertBundleTag("gc-live");
+  assert(GCLiveEntry->second == LLVMContext::OB_gc_live &&
+         "gc-transition operand bundle id drifted!");
+  (void)GCLiveEntry;
+
+  auto *ClangAttachedCall =
+      pImpl->getOrInsertBundleTag("clang.arc.attachedcall");
+  assert(ClangAttachedCall->second == LLVMContext::OB_clang_arc_attachedcall &&
+         "clang.arc.attachedcall operand bundle id drifted!");
+  (void)ClangAttachedCall;
+
+  auto *PtrauthEntry = pImpl->getOrInsertBundleTag("ptrauth");
+  assert(PtrauthEntry->second == LLVMContext::OB_ptrauth &&
+         "ptrauth operand bundle id drifted!");
+  (void)PtrauthEntry;
+
+  auto *KCFIEntry = pImpl->getOrInsertBundleTag("kcfi");
+  assert(KCFIEntry->second == LLVMContext::OB_kcfi &&
+         "kcfi operand bundle id drifted!");
+  (void)KCFIEntry;
+
+  auto *ConvergenceCtrlEntry = pImpl->getOrInsertBundleTag("convergencectrl");
+  assert(ConvergenceCtrlEntry->second == LLVMContext::OB_convergencectrl &&
+         "convergencectrl operand bundle id drifted!");
+  (void)ConvergenceCtrlEntry;
 
   SyncScope::ID SingleThreadSSID =
       pImpl->getOrInsertSyncScopeID("singlethread");

@arsenm
Copy link
Contributor

arsenm commented Dec 18, 2024

Can just remove the constexpr (or revert back to the original version which returned empty string)

@jplehr
Copy link
Contributor Author

jplehr commented Dec 18, 2024

Can just remove the constexpr (or revert back to the original version which returned empty string)

I don't have an opinion. Whatever is better.

@arsenm
Copy link
Contributor

arsenm commented Dec 18, 2024

Can just remove the constexpr (or revert back to the original version which returned empty string)

I don't have an opinion. Whatever is better.

Remove the constexpr, it's not doing much

@jplehr
Copy link
Contributor Author

jplehr commented Dec 18, 2024

I put up #120402

@arsenm
Copy link
Contributor

arsenm commented Dec 18, 2024

Should be obsoleted by #120402

@arsenm arsenm closed this Dec 18, 2024
@arsenm arsenm deleted the revert-120359-users/arsenm/llvmcontext-cleanup-bundle-registration branch December 18, 2024 14:15
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.

3 participants