From 9b81cc7b4c5a52529ee1c7180ec21af71b36e6ec Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 10 Jun 2025 20:15:26 -0700 Subject: [PATCH 1/5] Revert partial changes in 114660 This is due to a myriad of functional and performance issues with C++'s thread_local on various platforms. --- src/coreclr/debug/di/rsmain.cpp | 6 +++++- src/coreclr/debug/di/rspriv.h | 6 +++++- src/coreclr/pal/src/exception/seh.cpp | 9 +++++++-- src/coreclr/vm/amd64/AsmMacros.inc | 2 +- src/coreclr/vm/ceemain.cpp | 2 +- src/coreclr/vm/threads.inl | 2 +- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/coreclr/debug/di/rsmain.cpp b/src/coreclr/debug/di/rsmain.cpp index 33c9e82878c30e..582d7507926ccd 100644 --- a/src/coreclr/debug/di/rsmain.cpp +++ b/src/coreclr/debug/di/rsmain.cpp @@ -71,7 +71,11 @@ const char * GetDebugCodeName(DWORD dwCode) // Per-thread state for Debug builds... //----------------------------------------------------------------------------- #ifdef RSCONTRACTS -thread_local DbgRSThread* DbgRSThread::t_pCurrent; +#ifndef __GNUC__ +__declspec(thread) DbgRSThread* DbgRSThread::t_pCurrent; +#else // !__GNUC__ +__thread DbgRSThread* DbgRSThread::t_pCurrent; +#endif // !__GNUC__ LONG DbgRSThread::s_Total = 0; diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 3185cc0b554302..f8273a3963e908 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -11226,7 +11226,11 @@ class DbgRSThread DbgRSThread(); // The TLS slot that we'll put this thread object in. - static thread_local DbgRSThread* t_pCurrent; +#ifndef __GNUC__ + static __declspec(thread) DbgRSThread* t_pCurrent; +#else // !__GNUC__ + static __thread DbgRSThread* t_pCurrent; +#endif // !__GNUC__ static LONG s_Total; // Total count of thread objects diff --git a/src/coreclr/pal/src/exception/seh.cpp b/src/coreclr/pal/src/exception/seh.cpp index c2f28cff936c96..a5077e2fafa20c 100644 --- a/src/coreclr/pal/src/exception/seh.cpp +++ b/src/coreclr/pal/src/exception/seh.cpp @@ -174,7 +174,7 @@ PAL_ThrowExceptionFromContext(CONTEXT* context, PAL_SEHException* ex) // We need to make a copy of the exception off stack, since the "ex" is located in one of the stack // frames that will become obsolete by the ThrowExceptionFromContextInternal and the ThrowExceptionHelper // could overwrite the "ex" object by stack e.g. when allocating the low level exception object for "throw". - static thread_local BYTE threadLocalExceptionStorage[sizeof(PAL_SEHException)]; + static __thread BYTE threadLocalExceptionStorage[sizeof(PAL_SEHException)]; ThrowExceptionFromContextInternal(context, new (threadLocalExceptionStorage) PAL_SEHException(std::move(*ex))); } @@ -371,7 +371,12 @@ bool CatchHardwareExceptionHolder::IsEnabled() --*/ -static thread_local NativeExceptionHolderBase *t_nativeExceptionHolderHead = nullptr; +#if defined(__GNUC__) +static __thread +#else // __GNUC__ +__declspec(thread) static +#endif // !__GNUC__ +NativeExceptionHolderBase *t_nativeExceptionHolderHead = nullptr; extern "C" NativeExceptionHolderBase ** diff --git a/src/coreclr/vm/amd64/AsmMacros.inc b/src/coreclr/vm/amd64/AsmMacros.inc index 8d05620571257d..c6966135fc7ef4 100644 --- a/src/coreclr/vm/amd64/AsmMacros.inc +++ b/src/coreclr/vm/amd64/AsmMacros.inc @@ -467,7 +467,7 @@ OFFSETOF__ee_alloc_context EQU OFFSETOF__RuntimeThreadLocals__ee_alloc_context ; Pushes a TransitionBlock on the stack without saving the argument registers. See ; the PROLOG_WITH_TRANSITION_BLOCK macro for the stack layout. PUSH_COOP_PINVOKE_FRAME macro target - + __PWTB_StackAlloc = SIZEOF_MAX_OUTGOING_ARGUMENT_HOMES + 8 ; alignment to make the stack 16b aligned __PWTB_TransitionBlock = __PWTB_StackAlloc diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index bd00b1c7a5406d..841c4aa68bbbdf 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1664,7 +1664,7 @@ static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES; #define FLS_STATE_ARMED 1 #define FLS_STATE_INVOKED 2 -static thread_local byte t_flsState; +static __declspec(thread) byte t_flsState; // This is called when each *fiber* is destroyed. When the home fiber of a thread is destroyed, // it means that the thread itself is destroyed. diff --git a/src/coreclr/vm/threads.inl b/src/coreclr/vm/threads.inl index 8b59caaa10437e..16f235b3774d4e 100644 --- a/src/coreclr/vm/threads.inl +++ b/src/coreclr/vm/threads.inl @@ -27,7 +27,7 @@ EXTERN_C UINT32 _tls_index; #endif #ifdef _MSC_VER -__declspec(selectany) thread_local +__declspec(selectany) __declspec(thread) #else EXTERN_C __thread #endif From f1ac6795d6d8d693ad240737c362acd18c9f0495 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 11 Jun 2025 11:39:07 -0700 Subject: [PATCH 2/5] Create new PLATFORM_THREAD_LOCAL macro. Replace all explicit uses of platform thread local keywords. Address feedback. --- src/coreclr/debug/di/rsmain.cpp | 6 +----- src/coreclr/debug/di/rspriv.h | 6 +----- src/coreclr/pal/src/exception/seh.cpp | 9 ++------- src/coreclr/vm/ceemain.cpp | 2 +- src/coreclr/vm/common.h | 23 +++++++++++------------ src/coreclr/vm/jithelpers.cpp | 5 ++--- src/coreclr/vm/threads.cpp | 4 ++-- src/coreclr/vm/threads.h | 4 ++-- src/coreclr/vm/threads.inl | 6 +++--- src/coreclr/vm/threadstatics.h | 6 +++--- 10 files changed, 28 insertions(+), 43 deletions(-) diff --git a/src/coreclr/debug/di/rsmain.cpp b/src/coreclr/debug/di/rsmain.cpp index 582d7507926ccd..33c9e82878c30e 100644 --- a/src/coreclr/debug/di/rsmain.cpp +++ b/src/coreclr/debug/di/rsmain.cpp @@ -71,11 +71,7 @@ const char * GetDebugCodeName(DWORD dwCode) // Per-thread state for Debug builds... //----------------------------------------------------------------------------- #ifdef RSCONTRACTS -#ifndef __GNUC__ -__declspec(thread) DbgRSThread* DbgRSThread::t_pCurrent; -#else // !__GNUC__ -__thread DbgRSThread* DbgRSThread::t_pCurrent; -#endif // !__GNUC__ +thread_local DbgRSThread* DbgRSThread::t_pCurrent; LONG DbgRSThread::s_Total = 0; diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index f8273a3963e908..3185cc0b554302 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -11226,11 +11226,7 @@ class DbgRSThread DbgRSThread(); // The TLS slot that we'll put this thread object in. -#ifndef __GNUC__ - static __declspec(thread) DbgRSThread* t_pCurrent; -#else // !__GNUC__ - static __thread DbgRSThread* t_pCurrent; -#endif // !__GNUC__ + static thread_local DbgRSThread* t_pCurrent; static LONG s_Total; // Total count of thread objects diff --git a/src/coreclr/pal/src/exception/seh.cpp b/src/coreclr/pal/src/exception/seh.cpp index a5077e2fafa20c..c2f28cff936c96 100644 --- a/src/coreclr/pal/src/exception/seh.cpp +++ b/src/coreclr/pal/src/exception/seh.cpp @@ -174,7 +174,7 @@ PAL_ThrowExceptionFromContext(CONTEXT* context, PAL_SEHException* ex) // We need to make a copy of the exception off stack, since the "ex" is located in one of the stack // frames that will become obsolete by the ThrowExceptionFromContextInternal and the ThrowExceptionHelper // could overwrite the "ex" object by stack e.g. when allocating the low level exception object for "throw". - static __thread BYTE threadLocalExceptionStorage[sizeof(PAL_SEHException)]; + static thread_local BYTE threadLocalExceptionStorage[sizeof(PAL_SEHException)]; ThrowExceptionFromContextInternal(context, new (threadLocalExceptionStorage) PAL_SEHException(std::move(*ex))); } @@ -371,12 +371,7 @@ bool CatchHardwareExceptionHolder::IsEnabled() --*/ -#if defined(__GNUC__) -static __thread -#else // __GNUC__ -__declspec(thread) static -#endif // !__GNUC__ -NativeExceptionHolderBase *t_nativeExceptionHolderHead = nullptr; +static thread_local NativeExceptionHolderBase *t_nativeExceptionHolderHead = nullptr; extern "C" NativeExceptionHolderBase ** diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 841c4aa68bbbdf..bc776643ff1cb9 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1664,7 +1664,7 @@ static uint32_t g_flsIndex = FLS_OUT_OF_INDEXES; #define FLS_STATE_ARMED 1 #define FLS_STATE_INVOKED 2 -static __declspec(thread) byte t_flsState; +static PLATFORM_THREAD_LOCAL byte t_flsState; // This is called when each *fiber* is destroyed. When the home fiber of a thread is destroyed, // it means that the thread itself is destroyed. diff --git a/src/coreclr/vm/common.h b/src/coreclr/vm/common.h index d4be1f1f00d4a6..cc6514c69c5b3f 100644 --- a/src/coreclr/vm/common.h +++ b/src/coreclr/vm/common.h @@ -185,6 +185,17 @@ EXTERN_C Thread* STDCALL GetThreadHelper(); void SetThread(Thread*); +// Define a macro to declare TLS variables. There are scenarios +// where we want to use the platform-specific thread local storage +// mechanism: +// * It can have better performance characteristics than C++'s thread_local keyword. +// * Generally makes consuming these variables from assembly easier. +#ifdef _MSC_VER +#define PLATFORM_THREAD_LOCAL __declspec(thread) +#else +#define PLATFORM_THREAD_LOCAL __thread +#endif // _MSC_VER + // This is a mechanism by which macros can make the Thread pointer available to inner scopes // that is robust to code changes. If the outer Thread no longer is available for some reason // (e.g. code refactoring), this GET_THREAD() macro will fall back to calling GetThread(). @@ -207,18 +218,6 @@ EXTERN_C AppDomain* STDCALL GetAppDomain(); extern BOOL isMemoryReadable(const TADDR start, unsigned len); -#ifndef memcpyUnsafe_f -#define memcpyUnsafe_f - -// use this when you want to memcpy something that contains GC refs -FORCEINLINE void* memcpyUnsafe(void *dest, const void *src, size_t len) -{ - WRAPPER_NO_CONTRACT; - return memcpy(dest, src, len); -} - -#endif // !memcpyUnsafe_f - FORCEINLINE void* memcpyNoGCRefs(void * dest, const void * src, size_t len) { WRAPPER_NO_CONTRACT; diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 598e68fcc4467e..7f491adaa30618 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -349,10 +349,9 @@ HCIMPLEND // GetThreadLocalStaticBaseIfExistsAndInitialized function. // Using compiler specific thread local storage directives due to linkage issues. #ifdef _MSC_VER -__declspec(selectany) __declspec(thread) ThreadLocalData t_ThreadStatics; -#else -__thread ThreadLocalData t_ThreadStatics; +__declspec(selectany) #endif // _MSC_VER +PLATFORM_THREAD_LOCAL ThreadLocalData t_ThreadStatics; extern "C" void QCALLTYPE GetThreadStaticsByMethodTable(QCall::ByteRefOnStack refHandle, MethodTable* pMT, bool gcStatic) { diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 1834db25e7b421..3bc2038b7d52b2 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -72,7 +72,7 @@ TailCallTls::TailCallTls() } #ifndef _MSC_VER -__thread RuntimeThreadLocals t_runtime_thread_locals; +PLATFORM_THREAD_LOCAL RuntimeThreadLocals t_runtime_thread_locals; #endif Thread* STDCALL GetThreadHelper() @@ -345,7 +345,7 @@ bool Thread::DetectHandleILStubsForDebugger() } #ifndef _MSC_VER -__thread ThreadLocalInfo t_CurrentThreadInfo; +PLATFORM_THREAD_LOCAL ThreadLocalInfo t_CurrentThreadInfo; #endif // _MSC_VER #ifndef DACCESS_COMPILE diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index fa3d3342e903e2..3e142169c74c62 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -433,9 +433,9 @@ struct RuntimeThreadLocals #ifdef _MSC_VER // use selectany to avoid initialization de-optimization issues in the compiler -__declspec(selectany) thread_local +__declspec(selectany) PLATFORM_THREAD_LOCAL #else -extern __thread +extern PLATFORM_THREAD_LOCAL #endif RuntimeThreadLocals t_runtime_thread_locals; diff --git a/src/coreclr/vm/threads.inl b/src/coreclr/vm/threads.inl index 16f235b3774d4e..1cf28d27c6dc42 100644 --- a/src/coreclr/vm/threads.inl +++ b/src/coreclr/vm/threads.inl @@ -27,11 +27,11 @@ EXTERN_C UINT32 _tls_index; #endif #ifdef _MSC_VER -__declspec(selectany) __declspec(thread) +__declspec(selectany) #else -EXTERN_C __thread +EXTERN_C #endif -ThreadLocalInfo t_CurrentThreadInfo; +PLATFORM_THREAD_LOCAL ThreadLocalInfo t_CurrentThreadInfo; inline Thread* GetThreadNULLOk() { diff --git a/src/coreclr/vm/threadstatics.h b/src/coreclr/vm/threadstatics.h index 9bcc4e9a6a5f13..b114555982de46 100644 --- a/src/coreclr/vm/threadstatics.h +++ b/src/coreclr/vm/threadstatics.h @@ -87,11 +87,11 @@ typedef DPTR(ThreadLocalData) PTR_ThreadLocalData; // Using compiler specific thread local storage directives due to linkage issues. #ifndef DACCESS_COMPILE +extern #ifdef _MSC_VER -extern __declspec(selectany) __declspec(thread) ThreadLocalData t_ThreadStatics; -#else -extern __thread ThreadLocalData t_ThreadStatics; +__declspec(selectany) #endif // _MSC_VER +PLATFORM_THREAD_LOCAL ThreadLocalData t_ThreadStatics; #endif // !DACCESS_COMPILE #define NUMBER_OF_TLSOFFSETS_NOT_USED_IN_NONCOLLECTIBLE_ARRAY 2 From acb0a2c537914551371e9606e7491e42017d254f Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 11 Jun 2025 11:41:24 -0700 Subject: [PATCH 3/5] Remove redundancy. --- src/coreclr/vm/threads.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 3e142169c74c62..25b50e2e97f9b6 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -433,11 +433,11 @@ struct RuntimeThreadLocals #ifdef _MSC_VER // use selectany to avoid initialization de-optimization issues in the compiler -__declspec(selectany) PLATFORM_THREAD_LOCAL +__declspec(selectany) #else -extern PLATFORM_THREAD_LOCAL +extern #endif -RuntimeThreadLocals t_runtime_thread_locals; +PLATFORM_THREAD_LOCAL RuntimeThreadLocals t_runtime_thread_locals; typedef DPTR(struct RuntimeThreadLocals) PTR_RuntimeThreadLocals; typedef DPTR(struct gc_alloc_context) PTR_gc_alloc_context; From 1e4c9b5cc0fa3423a71700de768cffc4cbe49a20 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 12 Jun 2025 10:59:53 -0700 Subject: [PATCH 4/5] Move macros to native/minipal to share with native AOT. --- src/coreclr/debug/ee/debugger.h | 2 +- src/coreclr/nativeaot/Runtime/CommonMacros.h | 6 ------ src/coreclr/nativeaot/Runtime/GCHelpers.cpp | 2 +- src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp | 8 ++------ src/coreclr/nativeaot/Runtime/threadstore.cpp | 2 +- src/coreclr/nativeaot/Runtime/threadstore.inl | 6 +++--- src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp | 6 +++--- src/coreclr/vm/common.h | 11 ----------- src/coreclr/vm/threads.h | 10 +++++----- src/native/minipal/utils.h | 11 +++++++++++ 10 files changed, 27 insertions(+), 37 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 89999fac659592..f8f003d70bb573 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -3835,7 +3835,7 @@ HANDLE OpenWin32EventOrThrow( #define SENDIPCEVENT_RAW_END SENDIPCEVENT_RAW_END_EX // Suspend-aware SENDIPCEVENT macros: -// Check whether __thread has been suspended by the debugger via SetDebugState(). +// Check whether thread has been suspended by the debugger via SetDebugState(). // If this thread has been suspended, it shouldn't send any event to the RS because the // debugger may not be expecting it. Instead, just leave the lock and retry. // When we leave, we'll enter coop mode first and get suspended if a suspension is in progress. diff --git a/src/coreclr/nativeaot/Runtime/CommonMacros.h b/src/coreclr/nativeaot/Runtime/CommonMacros.h index a7e3dbd57cf651..157f5ba824615d 100644 --- a/src/coreclr/nativeaot/Runtime/CommonMacros.h +++ b/src/coreclr/nativeaot/Runtime/CommonMacros.h @@ -335,12 +335,6 @@ extern uint64_t g_startupTimelineEvents[NUM_STARTUP_TIMELINE_EVENTS]; #define C_ASSERT(e) static_assert(e, #e) #endif // C_ASSERT -#ifdef _MSC_VER -#define DECLSPEC_THREAD __declspec(thread) -#else // _MSC_VER -#define DECLSPEC_THREAD __thread -#endif // !_MSC_VER - // PAL Numbers // Used to ensure cross-compiler compatibility when declaring large // integer constants. 64-bit integer constants should be wrapped in the diff --git a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp index 6e37321143e541..637a92994d6bc6 100644 --- a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp @@ -405,7 +405,7 @@ FCIMPLEND // The MethodTable is remembered in some slow-path allocation paths. This value is used in event tracing. // It may statistically correlate with the most allocated type on the given stack/thread. -DECLSPEC_THREAD +PLATFORM_THREAD_LOCAL MethodTable* tls_pLastAllocationEEType = NULL; MethodTable* GetLastAllocEEType() diff --git a/src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp b/src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp index 8d2c4b0e7c68d0..f42f8d524d7b44 100644 --- a/src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp +++ b/src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp @@ -50,7 +50,7 @@ ep_rt_aot_walk_managed_stack_for_thread ( EP_ASSERT (thread != NULL); EP_ASSERT (stack_contents != NULL); - StackFrameIterator frameIterator(thread, thread->GetTransitionFrameForSampling()); + StackFrameIterator frameIterator(thread, thread->GetTransitionFrameForSampling()); while (frameIterator.IsValid()) { @@ -223,11 +223,7 @@ ep_rt_aot_diagnostics_command_line_get (void) namespace { - #ifdef TARGET_UNIX - __thread EventPipeThreadHolder* eventpipe_tls_instance; - #else - thread_local EventPipeThreadHolder* eventpipe_tls_instance; - #endif + PLATFORM_THREAD_LOCAL EventPipeThreadHolder* eventpipe_tls_instance; void free_thread_holder () { diff --git a/src/coreclr/nativeaot/Runtime/threadstore.cpp b/src/coreclr/nativeaot/Runtime/threadstore.cpp index 2dc0c1a64d9738..daf745ba0b65ef 100644 --- a/src/coreclr/nativeaot/Runtime/threadstore.cpp +++ b/src/coreclr/nativeaot/Runtime/threadstore.cpp @@ -418,7 +418,7 @@ FCIMPLEND C_ASSERT(sizeof(Thread) == sizeof(RuntimeThreadLocals)); #ifndef _MSC_VER -__thread RuntimeThreadLocals tls_CurrentThread; +PLATFORM_THREAD_LOCAL RuntimeThreadLocals tls_CurrentThread; #endif EXTERN_C RuntimeThreadLocals* RhpGetThread() diff --git a/src/coreclr/nativeaot/Runtime/threadstore.inl b/src/coreclr/nativeaot/Runtime/threadstore.inl index 5b4701249eec4b..4be81a7e60891b 100644 --- a/src/coreclr/nativeaot/Runtime/threadstore.inl +++ b/src/coreclr/nativeaot/Runtime/threadstore.inl @@ -1,12 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +EXTERN_C #ifdef _MSC_VER // a workaround to prevent tls_CurrentThread from becoming dynamically checked/initialized. -EXTERN_C __declspec(selectany) __declspec(thread) RuntimeThreadLocals tls_CurrentThread; -#else -EXTERN_C __thread RuntimeThreadLocals tls_CurrentThread; +__declspec(selectany) #endif +PLATFORM_THREAD_LOCAL RuntimeThreadLocals tls_CurrentThread; // static inline Thread * ThreadStore::RawGetCurrentThread() diff --git a/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp b/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp index ef025469fb9170..a2afb73d8abe35 100644 --- a/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp +++ b/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp @@ -481,7 +481,7 @@ thread_local TlsDestructionMonitor tls_destructionMonitor; #endif // This thread local variable is used for delegate marshalling -DECLSPEC_THREAD intptr_t tls_thunkData; +PLATFORM_THREAD_LOCAL intptr_t tls_thunkData; #ifdef FEATURE_EMULATED_TLS EXTERN_C intptr_t* RhpGetThunkData() @@ -1061,8 +1061,8 @@ int32_t PalGetProcessCpuCount() return g_RhNumberOfProcessors; } -__thread void* pStackHighOut = NULL; -__thread void* pStackLowOut = NULL; +PLATFORM_THREAD_LOCAL void* pStackHighOut = NULL; +PLATFORM_THREAD_LOCAL void* pStackLowOut = NULL; // Retrieves the entire range of memory dedicated to the calling thread's stack. This does // not get the current dynamic bounds of the stack, which can be significantly smaller than diff --git a/src/coreclr/vm/common.h b/src/coreclr/vm/common.h index cc6514c69c5b3f..bb587124cf168f 100644 --- a/src/coreclr/vm/common.h +++ b/src/coreclr/vm/common.h @@ -185,17 +185,6 @@ EXTERN_C Thread* STDCALL GetThreadHelper(); void SetThread(Thread*); -// Define a macro to declare TLS variables. There are scenarios -// where we want to use the platform-specific thread local storage -// mechanism: -// * It can have better performance characteristics than C++'s thread_local keyword. -// * Generally makes consuming these variables from assembly easier. -#ifdef _MSC_VER -#define PLATFORM_THREAD_LOCAL __declspec(thread) -#else -#define PLATFORM_THREAD_LOCAL __thread -#endif // _MSC_VER - // This is a mechanism by which macros can make the Thread pointer available to inner scopes // that is robust to code changes. If the outer Thread no longer is available for some reason // (e.g. code refactoring), this GET_THREAD() macro will fall back to calling GetThread(). diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 25b50e2e97f9b6..9f177c67c8f24c 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -331,13 +331,13 @@ void DestroyThread(Thread *th); DWORD GetRuntimeId(); -#define CREATETHREAD_IF_NULL_FAILFAST(__thread, __msg) \ +#define CREATETHREAD_IF_NULL_FAILFAST(thread__, msg__) \ { \ - HRESULT __ctinffhr; \ - __thread = SetupThreadNoThrow(&__ctinffhr); \ - if (__thread == NULL) \ + HRESULT ctinffhr__; \ + thread__ = SetupThreadNoThrow(&ctinffhr__); \ + if (thread__ == NULL) \ { \ - EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(__ctinffhr, __msg); \ + EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(ctinffhr__, msg__); \ UNREACHABLE(); \ } \ } diff --git a/src/native/minipal/utils.h b/src/native/minipal/utils.h index de25c457341e4f..ca8a1386fa83c1 100644 --- a/src/native/minipal/utils.h +++ b/src/native/minipal/utils.h @@ -44,6 +44,17 @@ #define LIBC_CALLBACK #endif +// Define a macro to declare TLS variables. There are cases +// where we want to use the platform specific thread-local +// storage mechanism: +// * Better performance characteristics compared to C++'s thread_local keyword. +// * Makes consuming TLS variables from assembly code easier. +#ifdef defined(_MSC_VER) +#define PLATFORM_THREAD_LOCAL __declspec(thread) +#else +#define PLATFORM_THREAD_LOCAL __thread +#endif + #if defined(_MSC_VER) # if defined(__SANITIZE_ADDRESS__) # define HAS_ADDRESS_SANITIZER From 2f6e5fe33d2ec27ab7fe3192d7bd602ec5a30f19 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 12 Jun 2025 13:06:18 -0700 Subject: [PATCH 5/5] Fix build break Apply feedback --- src/coreclr/nativeaot/Runtime/GCHelpers.cpp | 2 +- .../nativeaot/Runtime/unix/PalUnix.cpp | 43 +++++++++---------- src/coreclr/vm/jithelpers.cpp | 3 -- src/native/minipal/utils.h | 2 +- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp index 637a92994d6bc6..e7fdf907977b32 100644 --- a/src/coreclr/nativeaot/Runtime/GCHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/GCHelpers.cpp @@ -405,7 +405,7 @@ FCIMPLEND // The MethodTable is remembered in some slow-path allocation paths. This value is used in event tracing. // It may statistically correlate with the most allocated type on the given stack/thread. -PLATFORM_THREAD_LOCAL +static PLATFORM_THREAD_LOCAL MethodTable* tls_pLastAllocationEEType = NULL; MethodTable* GetLastAllocEEType() diff --git a/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp b/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp index a2afb73d8abe35..74c709e922b31a 100644 --- a/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp +++ b/src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp @@ -1061,48 +1061,45 @@ int32_t PalGetProcessCpuCount() return g_RhNumberOfProcessors; } -PLATFORM_THREAD_LOCAL void* pStackHighOut = NULL; -PLATFORM_THREAD_LOCAL void* pStackLowOut = NULL; - // Retrieves the entire range of memory dedicated to the calling thread's stack. This does // not get the current dynamic bounds of the stack, which can be significantly smaller than // the maximum bounds. bool PalGetMaximumStackBounds(_Out_ void** ppStackLowOut, _Out_ void** ppStackHighOut) { - if (pStackHighOut == NULL) - { + void* pStackHighOut = NULL; + void* pStackLowOut = NULL; + #ifdef __APPLE__ - // This is a Mac specific method - pStackHighOut = pthread_get_stackaddr_np(pthread_self()); - pStackLowOut = ((uint8_t *)pStackHighOut - pthread_get_stacksize_np(pthread_self())); + // This is a Mac specific method + pStackHighOut = pthread_get_stackaddr_np(pthread_self()); + pStackLowOut = ((uint8_t *)pStackHighOut - pthread_get_stacksize_np(pthread_self())); #else // __APPLE__ - pthread_attr_t attr; - size_t stackSize; - int status; + pthread_attr_t attr; + size_t stackSize; + int status; - pthread_t thread = pthread_self(); + pthread_t thread = pthread_self(); - status = pthread_attr_init(&attr); - ASSERT_MSG(status == 0, "pthread_attr_init call failed"); + status = pthread_attr_init(&attr); + ASSERT_MSG(status == 0, "pthread_attr_init call failed"); #if HAVE_PTHREAD_ATTR_GET_NP - status = pthread_attr_get_np(thread, &attr); + status = pthread_attr_get_np(thread, &attr); #elif HAVE_PTHREAD_GETATTR_NP - status = pthread_getattr_np(thread, &attr); + status = pthread_getattr_np(thread, &attr); #else #error Dont know how to get thread attributes on this platform! #endif - ASSERT_MSG(status == 0, "pthread_getattr_np call failed"); + ASSERT_MSG(status == 0, "pthread_getattr_np call failed"); - status = pthread_attr_getstack(&attr, &pStackLowOut, &stackSize); - ASSERT_MSG(status == 0, "pthread_attr_getstack call failed"); + status = pthread_attr_getstack(&attr, &pStackLowOut, &stackSize); + ASSERT_MSG(status == 0, "pthread_attr_getstack call failed"); - status = pthread_attr_destroy(&attr); - ASSERT_MSG(status == 0, "pthread_attr_destroy call failed"); + status = pthread_attr_destroy(&attr); + ASSERT_MSG(status == 0, "pthread_attr_destroy call failed"); - pStackHighOut = (uint8_t*)pStackLowOut + stackSize; + pStackHighOut = (uint8_t*)pStackLowOut + stackSize; #endif // __APPLE__ - } *ppStackLowOut = pStackLowOut; *ppStackHighOut = pStackHighOut; diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 7f491adaa30618..74f6adc2c8b4a5 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -344,9 +344,6 @@ HCIMPLEND // //======================================================================== -// Define the t_ThreadStatics variable here, so that these helpers can use -// the most optimal TLS access pattern for the platform when inlining the -// GetThreadLocalStaticBaseIfExistsAndInitialized function. // Using compiler specific thread local storage directives due to linkage issues. #ifdef _MSC_VER __declspec(selectany) diff --git a/src/native/minipal/utils.h b/src/native/minipal/utils.h index ca8a1386fa83c1..23a6a643601d1e 100644 --- a/src/native/minipal/utils.h +++ b/src/native/minipal/utils.h @@ -49,7 +49,7 @@ // storage mechanism: // * Better performance characteristics compared to C++'s thread_local keyword. // * Makes consuming TLS variables from assembly code easier. -#ifdef defined(_MSC_VER) +#if defined(_MSC_VER) #define PLATFORM_THREAD_LOCAL __declspec(thread) #else #define PLATFORM_THREAD_LOCAL __thread