[libcxx-commits] [PATCH] D133377: [libc++] Always query the compiler to find whether a type is always lockfree

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked an inline comment as done.
ldionne added a comment.

In D133377#3774759 <https://reviews.llvm.org/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.



================
Comment at: libcxx/include/atomic:1408
 
-#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<__libcpp_is_always_lock_free<_Tp>::__value,
----------------
Mordante wrote:
> 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 ;-)
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133377/new/

https://reviews.llvm.org/D133377