[llvm-branch-commits] [libcxx] d64394b - [libc++] Always query the compiler to find whether a type is always lockfree

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Sep 19 12:48:07 PDT 2022


Author: Louis Dionne
Date: 2022-09-19T21:47:55+02:00
New Revision: d64394b81d6e9d7a97a08fb1c6b42a24d2847b6c

URL: https://github.com/llvm/llvm-project/commit/d64394b81d6e9d7a97a08fb1c6b42a24d2847b6c
DIFF: https://github.com/llvm/llvm-project/commit/d64394b81d6e9d7a97a08fb1c6b42a24d2847b6c.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

(cherry picked from commit f1a601fe88f99d52ca80617266897b217bcd4d64)

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 76a227b5c6d07..bbc6063e5fbfa 100644
--- a/libcxx/docs/ReleaseNotes.rst
+++ b/libcxx/docs/ReleaseNotes.rst
@@ -194,6 +194,14 @@ Upcoming Deprecations and Removals
 
 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.
 
 - The ``_LIBCPP_ABI_USE_CXX03_NULLPTR_EMULATION`` macro controlling whether we use an
   emulation for ``std::nullptr_t`` in C++03 mode has been removed. After this change,

diff  --git a/libcxx/include/atomic b/libcxx/include/atomic
index 92da4820e928b..3a93b9b0a1081 100644
--- a/libcxx/include/atomic
+++ b/libcxx/include/atomic
@@ -1113,6 +1113,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>
@@ -1404,42 +1410,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
@@ -1561,7 +1533,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
@@ -2664,7 +2636,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 3c8efde1deb4f..e99af02e4f8e2 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 5b9c0dc8a904c..de24bc2580ec6 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 llvm-branch-commits mailing list