[libc++] Always query the compiler to find whether a type is always lockfree

[ldionne]

Summary

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

[ldionne]

This is for initial comments. I will figure out a way to test this tomorrow. We'll also need to discuss whether we should cherry-pick to LLVM 15.

I think we should, since we should fix the ABI break as soon as feasible. @thieta Any opinion? FWIW, this only concerns freestanding users, so this is not as widespread as it may seem at first sight.

[ldionne]

Add tests.

[thieta]

When you say freestanding: what does that mean? Ping-pong in the ABI is never great - but if it's not for all users maybe it's worth doing if we think it'll be stable going forward. Do you think it would warrant a 15.1.0 instead of a 15.0.1 in that case?

[Mordante]

Thanks for picking this up! In general happy with it, but some minor issue.

libcxx/include/atomic

1112

Can you update the release notes for this change.

1409

Can you add a comment why we use the C++17 feature here unconditionally?
This looks like a bug and surely somebody will try to fix it in 5 years time ;-)

[ldionne]

In D133377#3774759, @thieta wrote:

When you say freestanding: what does that mean?

I mean users literally passing the -ffreestanding flag. This is used on embedded platforms, and it's generally a much smaller audience than the mainstream development communities. Not to downplay the importance of it at all, quite the opposite, however in terms of impact this should reach much fewer developers.

Ping-pong in the ABI is never great - but if it's not for all users maybe it's worth doing if we think it'll be stable going forward. Do you think it would warrant a 15.1.0 instead of a 15.0.1 in that case?

Well, I guess the situation is really that we introduced a bug which made our ABI very sub-optimal (i.e. there's a lock in the atomic even when it could be lock free). I think it is definitely worth solving. I don't think it is necessary to do a 15.1.0, I'll just call it out in the release notes.

libcxx/include/atomic

1409

I am not sure why it looks like a bug? I'll add a comment to the definition of __libcpp_is_always_lock_free explaining that the compiler builtin we're using there is available in all versions.

[ldionne]

Address comments.

libcxx/docs/ReleaseNotes.rst

- The comparison operators on ``thread::id`` are now defined as free-standing
   Also see `issue 56187 <https://github.com/llvm/llvm-project/issues/56187>`_.
 
 - ``_LIBCPP_ENABLE_NODISCARD`` and ``_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17`` are no longer respected.
   Any standards-required ``[[nodiscard]]`` applications in C++20 are now always enabled. Any extended applications
   are now enabled by default and can be disabled by defining ``_LIBCPP_DISABLE_NODISCARD_EXT``.
 
 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
 --------------------

libcxx/include/atomic

 # define ATOMIC_WCHAR_T_LOCK_FREE   __GCC_ATOMIC_WCHAR_T_LOCK_FREE
 # define ATOMIC_SHORT_LOCK_FREE     __GCC_ATOMIC_SHORT_LOCK_FREE
 # define ATOMIC_INT_LOCK_FREE       __GCC_ATOMIC_INT_LOCK_FREE
 # define ATOMIC_LONG_LOCK_FREE      __GCC_ATOMIC_LONG_LOCK_FREE
 # define ATOMIC_LLONG_LOCK_FREE     __GCC_ATOMIC_LLONG_LOCK_FREE
 # 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>
 struct __cxx_atomic_lock_impl {
 
   _LIBCPP_INLINE_VISIBILITY
   __cxx_atomic_lock_impl() _NOEXCEPT
     : __a_value(), __a_lock(0) {}
_Tp __cxx_atomic_fetch_xor(__cxx_atomic_lock_impl<_Tp>* __a,
                            _Tp __pattern, memory_order) {
   __a->__lock();
   _Tp __old = __a->__a_value;
   __a->__a_value ^= __pattern;
   __a->__unlock();
   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
 template <typename _Tp,
           typename _Base = __cxx_atomic_base_impl<_Tp> >
 #endif //_LIBCPP_ATOMIC_ONLY_USE_BUILTINS
 struct __cxx_atomic_impl : public _Base {
     static_assert(is_trivially_copyable<_Tp>::value,

 // general atomic<T>
 
 template <class _Tp, bool = is_integral<_Tp>::value && !is_same<_Tp, bool>::value>
 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
     bool is_lock_free() const volatile _NOEXCEPT
         {return __cxx_atomic_is_lock_free(sizeof(_Tp));}
     _LIBCPP_INLINE_VISIBILITY
     bool is_lock_free() const _NOEXCEPT
         {return static_cast<__atomic_base const volatile*>(this)->is_lock_free();}

 typedef atomic<size_t>    atomic_size_t;
 typedef atomic<ptrdiff_t> atomic_ptrdiff_t;
 typedef atomic<intmax_t>  atomic_intmax_t;
 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
 
 #if ATOMIC_LLONG_LOCK_FREE == 2
 typedef conditional<_LIBCPP_CONTENTION_LOCK_FREE, __cxx_contention_t, long long>::type          __libcpp_signed_lock_free;
 typedef conditional<_LIBCPP_CONTENTION_LOCK_FREE, __cxx_contention_t, unsigned long long>::type __libcpp_unsigned_lock_free;
 #elif ATOMIC_INT_LOCK_FREE == 2

libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp

#endif
   CHECK_ALIGNMENT(struct OneInt { int i; });
   CHECK_ALIGNMENT(struct IntArr2 { int i[2]; });
   CHECK_ALIGNMENT(struct LLIArr2 { long long int i[2]; });
   CHECK_ALIGNMENT(struct LLIArr4 { long long int i[4]; });
   CHECK_ALIGNMENT(struct LLIArr8 { long long int i[8]; });
   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;
 }

libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp

 
 // static constexpr bool is_always_lock_free;
 
 #include <atomic>
 #include <cassert>
 
 #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()
 {
 // structs and unions can't be defined in the template invocation.
 // Work around this with a typedef.
 #define CHECK_ALWAYS_LOCK_FREE(T)                                              \
   do {                                                                         \
     typedef T type;                                                            \
#endif // __has_attribute(vector_size) && defined(_LIBCPP_VERSION)
     CHECK_ALWAYS_LOCK_FREE(struct OneInt { int i; });
     CHECK_ALWAYS_LOCK_FREE(struct IntArr2 { int i[2]; });
     CHECK_ALWAYS_LOCK_FREE(struct LLIArr2 { long long int i[2]; });
     CHECK_ALWAYS_LOCK_FREE(struct LLIArr4 { long long int i[4]; });
     CHECK_ALWAYS_LOCK_FREE(struct LLIArr8 { long long int i[8]; });
     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)
     static_assert(std::atomic<char8_t>::is_always_lock_free == (2 == ATOMIC_CHAR8_T_LOCK_FREE), "");
 #endif
     static_assert(std::atomic<char16_t>::is_always_lock_free == (2 == ATOMIC_CHAR16_T_LOCK_FREE), "");
     static_assert(std::atomic<char32_t>::is_always_lock_free == (2 == ATOMIC_CHAR32_T_LOCK_FREE), "");
     static_assert(std::atomic<wchar_t>::is_always_lock_free == (2 == ATOMIC_WCHAR_T_LOCK_FREE), "");