Skip to content

Conversation

keithw
Copy link

@keithw keithw commented Nov 12, 2023

This PR lets lld be used as a library in single-threaded environments without TLS.

@llvmbot llvmbot added the lld label Nov 12, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2023

@llvm/pr-subscribers-lld

Author: Keith Winstein (keithw)

Changes

This PR lets lld be used as a library in single-threaded environments without TLS.


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

1 Files Affected:

  • (modified) lld/include/lld/Common/Memory.h (+4)
diff --git a/lld/include/lld/Common/Memory.h b/lld/include/lld/Common/Memory.h
index c7612a08c6b00db..23e8737bcc207be 100644
--- a/lld/include/lld/Common/Memory.h
+++ b/lld/include/lld/Common/Memory.h
@@ -65,7 +65,11 @@ template <typename T, typename... U> T *make(U &&... args) {
 template <typename T>
 inline llvm::SpecificBumpPtrAllocator<T> &
 getSpecificAllocSingletonThreadLocal() {
+  #if LLVM_ENABLE_THREADS
   thread_local SpecificAlloc<T> instance;
+  #else
+  static SpecificAlloc<T> instance;
+  #endif
   return instance.alloc;
 }
 

Copy link

github-actions bot commented Nov 12, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Co-authored-by: Keith Winstein <keithw@cs.stanford.edu>
@keithw keithw force-pushed the lld-conditional-thread-local branch from b5a99da to d6ad22a Compare November 14, 2023 02:41
@keithw
Copy link
Author

keithw commented Nov 15, 2023

@smithp35 @MaskRay

@MaskRay
Copy link
Member

MaskRay commented Nov 27, 2023

Can you describe how is your single-threaded environment look like? Is thread_local an error for your compiler, linker, or loader?

It seems that we don't use thread_local for !LLVM_ENABLE_THREADS configurations in llvm/ and clang/ (but not mlir/) and I don't think it is good for the project if we add a lot of code in non-testable configurations (those with very special requirements).


static LLVM_THREAD_LOCAL should look better.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

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