[libcxx-commits] [libcxx] f1a601f - [libc++] Always query the compiler to find whether a type is always lockfree
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Sep 19 08:10:19 PDT 2022
Author: Louis Dionne
Date: 2022-09-19T11:10:02-04:00
New Revision: f1a601fe88f99d52ca80617266897b217bcd4d64
URL: https://github.com/llvm/llvm-project/commit/f1a601fe88f99d52ca80617266897b217bcd4d64
DIFF: https://github.com/llvm/llvm-project/commit/f1a601fe88f99d52ca80617266897b217bcd4d64.diff
LOG: [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
Added:
Modified:
libcxx/docs/ReleaseNotes.rst
libcxx/include/atomic
libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp
libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
Removed:
################################################################################
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)
More information about the libcxx-commits
mailing list