From 30bae52f11edc005cb4fb4ead443795f66fcfa1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Fri, 26 Jan 2024 00:28:29 +0100 Subject: [PATCH 1/6] Use non locking atomic intrinsics Makes PAL use intrinsics that are guaranteed to not lock with Clang and with other compilers (probably only GCC) checks the C11 `lock free atomics` defines as there seems to be no better way to verify safety there. Fixes #97452. --- src/coreclr/pal/inc/pal.h | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 7cfdf4d949968..a3c8dc2e53ce2 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3639,24 +3639,42 @@ Define_InterlockMethod( CHAR, InterlockedExchange8(IN OUT CHAR volatile *Target, CHAR Value), InterlockedExchange8(Target, Value), +#ifdef __clang__ + return __sync_swap(pDst, iValue); +#elif ATOMIC_CHAR_LOCK_FREE == 2 __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) +#else +#error Lock free atomic int8 support is required. +#endif ) Define_InterlockMethod( SHORT, InterlockedExchange16(IN OUT SHORT volatile *Target, SHORT Value), InterlockedExchange16(Target, Value), +#ifdef __clang__ + return __sync_swap(pDst, iValue); +#elif ATOMIC_SHORT_LOCK_FREE == 2 __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) +#else +#error Lock free atomic int16 support is required. +#endif ) Define_InterlockMethod( LONG, InterlockedExchange(IN OUT LONG volatile *Target, LONG Value), InterlockedExchange(Target, Value), +#ifdef __clang__ + return __sync_swap(pDst, iValue); +#elif ATOMIC_INT_LOCK_FREE == 2 __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) +#else +#error Lock free atomic int32 support is required. +#endif ) -#if defined(HOST_X86) +#if defined(HOST_X86) && defined(__clang__) // 64-bit __atomic_exchange_n is not expanded as a compiler intrinsic on Linux x86. // Use inline implementation instead. @@ -3678,12 +3696,17 @@ Define_InterlockMethod( LONGLONG, InterlockedExchange64(IN OUT LONGLONG volatile *Target, IN LONGLONG Value), InterlockedExchange64(Target, Value), +#ifdef __clang__ + return __sync_swap(pDst, iValue); +#elif ATOMIC_LLONG_LOCK_FREE == 2 __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) +#else +#error Lock free atomic int64 support is required. +#endif ) #endif - /*++ Function: InterlockedCompareExchange From 795a310a3b8cc9c593ecb7578de4c6fd5e86326c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Fri, 26 Jan 2024 00:31:19 +0100 Subject: [PATCH 2/6] Update pal.h --- src/coreclr/pal/inc/pal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index a3c8dc2e53ce2..a8a745cdb1438 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3674,7 +3674,7 @@ Define_InterlockMethod( #endif ) -#if defined(HOST_X86) && defined(__clang__) +#if defined(HOST_X86) && (defined(__clang__) || ATOMIC_LLONG_LOCK_FREE == 2) // 64-bit __atomic_exchange_n is not expanded as a compiler intrinsic on Linux x86. // Use inline implementation instead. From 92b65fb804505464b06cccfe7fa1f5f0c244f41d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Fri, 26 Jan 2024 02:07:29 +0100 Subject: [PATCH 3/6] Move checking to CMake --- src/coreclr/pal/inc/pal.h | 26 +++++++++----------------- src/coreclr/pal/src/config.h.in | 1 + src/coreclr/pal/src/configure.cmake | 21 +++++++++++++++++++++ 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index a8a745cdb1438..4e4287e834af8 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3640,11 +3640,9 @@ Define_InterlockMethod( InterlockedExchange8(IN OUT CHAR volatile *Target, CHAR Value), InterlockedExchange8(Target, Value), #ifdef __clang__ - return __sync_swap(pDst, iValue); -#elif ATOMIC_CHAR_LOCK_FREE == 2 - __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) + __sync_swap(Target, Value); #else -#error Lock free atomic int8 support is required. + __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) #endif ) @@ -3653,11 +3651,9 @@ Define_InterlockMethod( InterlockedExchange16(IN OUT SHORT volatile *Target, SHORT Value), InterlockedExchange16(Target, Value), #ifdef __clang__ - return __sync_swap(pDst, iValue); -#elif ATOMIC_SHORT_LOCK_FREE == 2 - __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) + __sync_swap(Target, Value); #else -#error Lock free atomic int16 support is required. + __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) #endif ) @@ -3666,15 +3662,13 @@ Define_InterlockMethod( InterlockedExchange(IN OUT LONG volatile *Target, LONG Value), InterlockedExchange(Target, Value), #ifdef __clang__ - return __sync_swap(pDst, iValue); -#elif ATOMIC_INT_LOCK_FREE == 2 - __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) + __sync_swap(Target, Value); #else -#error Lock free atomic int32 support is required. + __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) #endif ) -#if defined(HOST_X86) && (defined(__clang__) || ATOMIC_LLONG_LOCK_FREE == 2) +#if defined(HOST_X86) // 64-bit __atomic_exchange_n is not expanded as a compiler intrinsic on Linux x86. // Use inline implementation instead. @@ -3697,11 +3691,9 @@ Define_InterlockMethod( InterlockedExchange64(IN OUT LONGLONG volatile *Target, IN LONGLONG Value), InterlockedExchange64(Target, Value), #ifdef __clang__ - return __sync_swap(pDst, iValue); -#elif ATOMIC_LLONG_LOCK_FREE == 2 - __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) + __sync_swap(Target, Value); #else -#error Lock free atomic int64 support is required. + __atomic_exchange_n(Target, Value, __ATOMIC_ACQ_REL) #endif ) diff --git a/src/coreclr/pal/src/config.h.in b/src/coreclr/pal/src/config.h.in index b5ddd025f3eb1..e47aaa4d8a188 100644 --- a/src/coreclr/pal/src/config.h.in +++ b/src/coreclr/pal/src/config.h.in @@ -98,6 +98,7 @@ #cmakedefine01 HAVE__SC_PHYS_PAGES #cmakedefine01 HAVE__SC_AVPHYS_PAGES +#cmakedefine01 HAVE_LOCKFREE_ATOMICS #cmakedefine01 REALPATH_SUPPORTS_NONEXISTENT_FILES #cmakedefine01 SSCANF_SUPPORT_ll #cmakedefine01 HAVE_LARGE_SNPRINTF_SUPPORT diff --git a/src/coreclr/pal/src/configure.cmake b/src/coreclr/pal/src/configure.cmake index b8cdb3a4df413..b13ce497cce64 100644 --- a/src/coreclr/pal/src/configure.cmake +++ b/src/coreclr/pal/src/configure.cmake @@ -189,6 +189,27 @@ check_cxx_symbol_exists(_DEBUG sys/user.h USER_H_DEFINES_DEBUG) check_cxx_symbol_exists(_SC_PHYS_PAGES unistd.h HAVE__SC_PHYS_PAGES) check_cxx_symbol_exists(_SC_AVPHYS_PAGES unistd.h HAVE__SC_AVPHYS_PAGES) +check_cxx_source_runs(" +//#if defined(__clang__) || defined(_MSC_VER) +//int main(void) { +// exit(0); +//} +//#else +#include + +int main(void) { +#if ATOMIC_CHAR_LOCK_FREE == 2 && ATOMIC_SHORT_LOCK_FREE == 2 && ATOMIC_INT_LOCK_FREE == 2 && ATOMIC_LLONG_LOCK_FREE == 2 + exit(0); +#else + exit(1); +#endif +} +//#endif" HAVE_LOCKFREE_ATOMICS) + +if(NOT HAVE_LOCKFREE_ATOMICS) +message( FATAL_ERROR "CoreCLR requires non-locking atomics" ) +endif(NOT HAVE_LOCKFREE_ATOMICS) + check_cxx_source_runs(" #include #include From fa1d8191ebd18bb41181b53a71ee1678c371a2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Fri, 26 Jan 2024 02:20:06 +0100 Subject: [PATCH 4/6] Update configure.cmake --- src/coreclr/pal/src/configure.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/pal/src/configure.cmake b/src/coreclr/pal/src/configure.cmake index b13ce497cce64..844d31cd8277d 100644 --- a/src/coreclr/pal/src/configure.cmake +++ b/src/coreclr/pal/src/configure.cmake @@ -199,9 +199,9 @@ check_cxx_source_runs(" int main(void) { #if ATOMIC_CHAR_LOCK_FREE == 2 && ATOMIC_SHORT_LOCK_FREE == 2 && ATOMIC_INT_LOCK_FREE == 2 && ATOMIC_LLONG_LOCK_FREE == 2 - exit(0); + return 0; #else - exit(1); + return 1; #endif } //#endif" HAVE_LOCKFREE_ATOMICS) From c3cf1c4751e0916e58e1472a42a71a3c6f9f7fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Fri, 26 Jan 2024 02:36:39 +0100 Subject: [PATCH 5/6] Update configure.cmake --- src/coreclr/pal/src/configure.cmake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/pal/src/configure.cmake b/src/coreclr/pal/src/configure.cmake index 844d31cd8277d..6179f0938f2a3 100644 --- a/src/coreclr/pal/src/configure.cmake +++ b/src/coreclr/pal/src/configure.cmake @@ -189,10 +189,10 @@ check_cxx_symbol_exists(_DEBUG sys/user.h USER_H_DEFINES_DEBUG) check_cxx_symbol_exists(_SC_PHYS_PAGES unistd.h HAVE__SC_PHYS_PAGES) check_cxx_symbol_exists(_SC_AVPHYS_PAGES unistd.h HAVE__SC_AVPHYS_PAGES) -check_cxx_source_runs(" +check_c_source_compiles(" //#if defined(__clang__) || defined(_MSC_VER) //int main(void) { -// exit(0); +// return 0; //} //#else #include @@ -201,7 +201,7 @@ int main(void) { #if ATOMIC_CHAR_LOCK_FREE == 2 && ATOMIC_SHORT_LOCK_FREE == 2 && ATOMIC_INT_LOCK_FREE == 2 && ATOMIC_LLONG_LOCK_FREE == 2 return 0; #else - return 1; +#error CoreCLR requires non-locking atomics #endif } //#endif" HAVE_LOCKFREE_ATOMICS) From 492ed0e5046d7be1a52878120e885d6afcf2b1e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Fri, 26 Jan 2024 17:18:44 +0100 Subject: [PATCH 6/6] Make the code build --- src/coreclr/pal/src/configure.cmake | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/coreclr/pal/src/configure.cmake b/src/coreclr/pal/src/configure.cmake index 6179f0938f2a3..ac9702a2f60f9 100644 --- a/src/coreclr/pal/src/configure.cmake +++ b/src/coreclr/pal/src/configure.cmake @@ -190,21 +190,17 @@ check_cxx_symbol_exists(_SC_PHYS_PAGES unistd.h HAVE__SC_PHYS_PAGES) check_cxx_symbol_exists(_SC_AVPHYS_PAGES unistd.h HAVE__SC_AVPHYS_PAGES) check_c_source_compiles(" -//#if defined(__clang__) || defined(_MSC_VER) -//int main(void) { -// return 0; -//} -//#else #include int main(void) { -#if ATOMIC_CHAR_LOCK_FREE == 2 && ATOMIC_SHORT_LOCK_FREE == 2 && ATOMIC_INT_LOCK_FREE == 2 && ATOMIC_LLONG_LOCK_FREE == 2 +#if defined(__clang__) && __has_builtin(__sync_swap) && __has_builtin(__sync_val_compare_and_swap) + return 0; +#elif ATOMIC_CHAR_LOCK_FREE == 2 && ATOMIC_SHORT_LOCK_FREE == 2 && ATOMIC_INT_LOCK_FREE == 2 && ATOMIC_LLONG_LOCK_FREE == 2 return 0; #else #error CoreCLR requires non-locking atomics #endif -} -//#endif" HAVE_LOCKFREE_ATOMICS) +}" HAVE_LOCKFREE_ATOMICS) if(NOT HAVE_LOCKFREE_ATOMICS) message( FATAL_ERROR "CoreCLR requires non-locking atomics" )