diff options
author | Louis Dionne <ldionne.2@gmail.com> | 2022-09-07 00:07:18 +0300 |
---|---|---|
committer | Louis Dionne <ldionne.2@gmail.com> | 2022-09-19 18:10:02 +0300 |
commit | f1a601fe88f99d52ca80617266897b217bcd4d64 (patch) | |
tree | 641683e6761d36ec457510475ebd6ed51af3d597 /libcxx | |
parent | 0a4c946abcfaae3601c49a55e29944287f867b7f (diff) |
[libc++] Always query the compiler to find whether a type is always lockfree
In https://llvm.org/D56913, we added an emulation for the __atomic_always_lock_free
compiler builtin when compiling in Freestanding mode. However, the emulation
did (and could not) give exactly the same answer as the compiler builtin,
which led to a potential ABI break for e.g. enum classes.
After speaking to the original author of D56913, we agree that the correct
behavior is to instead always use the compiler builtin, since that provides
a more accurate answer, and __atomic_always_lock_free is a purely front-end
builtin which doesn't require any runtime support. Furthermore, it is
available regardless of the Standard mode (see https://godbolt.org/z/cazf3ssYY).
However, this patch does constitute an ABI break. As shown by https://godbolt.org/z/1eoex6zdK:
- In LLVM <= 11.0.1, an atomic<enum class with 1 byte> would not contain a lock byte.
- In LLVM >= 12.0.0, an atomic<enum class with 1 byte> would contain a lock byte.
This patch breaks the ABI again to bring it back to 1 byte, which seems
like the correct thing to do.
Fixes #57440
Differential Revision: https://reviews.llvm.org/D133377
Diffstat (limited to 'libcxx')
-rw-r--r-- | libcxx/docs/ReleaseNotes.rst | 8 | ||||
-rw-r--r-- | libcxx/include/atomic | 46 | ||||
-rw-r--r-- | libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp | 1 | ||||
-rw-r--r-- | libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp | 14 |
4 files changed, 26 insertions, 43 deletions
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst index 2f0fb9d88d95..c01332696218 100644 --- a/libcxx/docs/ReleaseNotes.rst +++ b/libcxx/docs/ReleaseNotes.rst @@ -85,6 +85,14 @@ API Changes ABI Affecting Changes --------------------- +- In freestanding mode, ``atomic<small enum class>`` does not contain a lock byte anymore if the platform + can implement lockfree atomics for that size. More specifically, in LLVM <= 11.0.1, an ``atomic<small enum class>`` + would not contain a lock byte. This was broken in LLVM >= 12.0.0, where it started including a lock byte despite + the platform supporting lockfree atomics for that size. Starting in LLVM 15.0.1, the ABI for these types has been + restored to what it used to be (no lock byte), which is the most efficient implementation. + + This ABI break only affects users that compile with ``-ffreestanding``, and only for ``atomic<T>`` where ``T`` + is a non-builtin type that could be lockfree on the platform. See https://llvm.org/D133377 for more details. Build System Changes -------------------- diff --git a/libcxx/include/atomic b/libcxx/include/atomic index 4775022f7503..e16f9fdd3a0d 100644 --- a/libcxx/include/atomic +++ b/libcxx/include/atomic @@ -1109,6 +1109,12 @@ _Tp kill_dependency(_Tp __y) _NOEXCEPT # define ATOMIC_POINTER_LOCK_FREE __GCC_ATOMIC_POINTER_LOCK_FREE #endif +template <class _Tp> +struct __libcpp_is_always_lock_free { + // __atomic_always_lock_free is available in all Standard modes + static const bool __value = __atomic_always_lock_free(sizeof(_Tp), 0); +}; + #ifdef _LIBCPP_ATOMIC_ONLY_USE_BUILTINS template<typename _Tp> @@ -1400,42 +1406,8 @@ _Tp __cxx_atomic_fetch_xor(__cxx_atomic_lock_impl<_Tp>* __a, return __old; } -#ifdef __cpp_lib_atomic_is_always_lock_free - -template<typename _Tp> struct __cxx_is_always_lock_free { - enum { __value = __atomic_always_lock_free(sizeof(_Tp), 0) }; }; - -#else - -template<typename _Tp> struct __cxx_is_always_lock_free { enum { __value = false }; }; -// Implementations must match the C ATOMIC_*_LOCK_FREE macro values. -template<> struct __cxx_is_always_lock_free<bool> { enum { __value = 2 == ATOMIC_BOOL_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<char> { enum { __value = 2 == ATOMIC_CHAR_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<signed char> { enum { __value = 2 == ATOMIC_CHAR_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<unsigned char> { enum { __value = 2 == ATOMIC_CHAR_LOCK_FREE }; }; -#ifndef _LIBCPP_HAS_NO_CHAR8_T -template<> struct __cxx_is_always_lock_free<char8_t> { enum { __value = 2 == ATOMIC_CHAR8_T_LOCK_FREE }; }; -#endif -template<> struct __cxx_is_always_lock_free<char16_t> { enum { __value = 2 == ATOMIC_CHAR16_T_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<char32_t> { enum { __value = 2 == ATOMIC_CHAR32_T_LOCK_FREE }; }; -#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS -template<> struct __cxx_is_always_lock_free<wchar_t> { enum { __value = 2 == ATOMIC_WCHAR_T_LOCK_FREE }; }; -#endif -template<> struct __cxx_is_always_lock_free<short> { enum { __value = 2 == ATOMIC_SHORT_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<unsigned short> { enum { __value = 2 == ATOMIC_SHORT_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<int> { enum { __value = 2 == ATOMIC_INT_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<unsigned int> { enum { __value = 2 == ATOMIC_INT_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<long> { enum { __value = 2 == ATOMIC_LONG_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<unsigned long> { enum { __value = 2 == ATOMIC_LONG_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<long long> { enum { __value = 2 == ATOMIC_LLONG_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<unsigned long long> { enum { __value = 2 == ATOMIC_LLONG_LOCK_FREE }; }; -template<typename _Tp> struct __cxx_is_always_lock_free<_Tp*> { enum { __value = 2 == ATOMIC_POINTER_LOCK_FREE }; }; -template<> struct __cxx_is_always_lock_free<std::nullptr_t> { enum { __value = 2 == ATOMIC_POINTER_LOCK_FREE }; }; - -#endif //__cpp_lib_atomic_is_always_lock_free - template <typename _Tp, - typename _Base = typename conditional<__cxx_is_always_lock_free<_Tp>::__value, + typename _Base = typename conditional<__libcpp_is_always_lock_free<_Tp>::__value, __cxx_atomic_base_impl<_Tp>, __cxx_atomic_lock_impl<_Tp> >::type> #else @@ -1542,7 +1514,7 @@ struct __atomic_base // false mutable __cxx_atomic_impl<_Tp> __a_; #if defined(__cpp_lib_atomic_is_always_lock_free) - static _LIBCPP_CONSTEXPR bool is_always_lock_free = __atomic_always_lock_free(sizeof(__a_), 0); + static _LIBCPP_CONSTEXPR bool is_always_lock_free = __libcpp_is_always_lock_free<__cxx_atomic_impl<_Tp> >::__value; #endif _LIBCPP_INLINE_VISIBILITY @@ -2645,7 +2617,7 @@ typedef atomic<uintmax_t> atomic_uintmax_t; // atomic_*_lock_free : prefer the contention type most highly, then the largest lock-free type #ifdef __cpp_lib_atomic_is_always_lock_free -# define _LIBCPP_CONTENTION_LOCK_FREE __atomic_always_lock_free(sizeof(__cxx_contention_t), 0) +# define _LIBCPP_CONTENTION_LOCK_FREE ::std::__libcpp_is_always_lock_free<__cxx_contention_t>::__value #else # define _LIBCPP_CONTENTION_LOCK_FREE false #endif diff --git a/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp b/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp index 3c8efde1deb4..e99af02e4f8e 100644 --- a/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp +++ b/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp @@ -105,6 +105,7 @@ int main(int, char**) { CHECK_ALIGNMENT(struct LLIArr16 { long long int i[16]; }); CHECK_ALIGNMENT(struct Padding { char c; /* padding */ long long int i; }); CHECK_ALIGNMENT(union IntFloat { int i; float f; }); + CHECK_ALIGNMENT(enum class StrongEnum { foo }); return 0; } diff --git a/libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp b/libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp index 5b9c0dc8a904..de24bc2580ec 100644 --- a/libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp +++ b/libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp @@ -17,13 +17,12 @@ #include "test_macros.h" -#if !defined(__cpp_lib_atomic_is_always_lock_free) -# error Feature test macro missing. -#endif - -template <typename T> void checkAlwaysLockFree() { - if (std::atomic<T>::is_always_lock_free) +template <typename T> +void checkAlwaysLockFree() { + if (std::atomic<T>::is_always_lock_free) { + LIBCPP_ASSERT(sizeof(std::atomic<T>) == sizeof(T)); // technically not required, but libc++ does it that way assert(std::atomic<T>().is_lock_free()); + } } void run() @@ -85,10 +84,13 @@ void run() CHECK_ALWAYS_LOCK_FREE(struct LLIArr16 { long long int i[16]; }); CHECK_ALWAYS_LOCK_FREE(struct Padding { char c; /* padding */ long long int i; }); CHECK_ALWAYS_LOCK_FREE(union IntFloat { int i; float f; }); + CHECK_ALWAYS_LOCK_FREE(enum class CharEnumClass : char { foo }); // C macro and static constexpr must be consistent. + enum class CharEnumClass : char { foo }; static_assert(std::atomic<bool>::is_always_lock_free == (2 == ATOMIC_BOOL_LOCK_FREE), ""); static_assert(std::atomic<char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), ""); + static_assert(std::atomic<CharEnumClass>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), ""); static_assert(std::atomic<signed char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), ""); static_assert(std::atomic<unsigned char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), ""); #if TEST_STD_VER > 17 && defined(__cpp_char8_t) |