[libcxx-commits] [libcxx] [libc++] Implement C++20 atomic_ref (PR #76647)
James Y Knight via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Dec 31 08:26:51 PST 2023
jyknight wrote:
It's important to note that this PR deletes the codepath in cxx_atomic_impl.h which uses the `__c11_*` builtins and C `_Atomic(T)` to implement `std::atomic`.
That's a wonderful simplification, and I would _love_ to see it done. BUT, it changes the ABI for `std::atomic`. It both makes it incompatible with previous libc++, and with Clang's C11 `_Atomic(T)` type.
Unfortunately, the current situation is a total mess.
### Mess 1: _Atomic ABI
Clang's ABI for _Atomic(T) is complex: it attempts to round up both size and alignment of T, but only up to a per-platform limit, to ensure that a lock-free atomic can be used if possible. That is: given `struct X { char n[3] };`, on some platforms, `sizeof(std::atomic<X>)` and `alignof(std::atomic<X>)`, will be 4 and 4, but on others, it'll be 3 and 1. See the `MaxAtomicPromoteWidth` values set by clang/lib/Basic/Targets/*.
Which means C11 atomics in Clang are ABI-incompatible with GCC. GCC never adds padding, but instead simply increases alignment for objects of size 1, 2, 4, 8, 16 to match their size. Quite unfortunate situation, since usually GCC and Clang are ABI-compatible for C code.
So: the ABI for C11 _Atomic differs between Clang and GCC. The ABI for libc++'s std::atomic _also_ changes depending on which compiler you build with (since it changes which underlying implementation strategy is used). It matches C11 _Atomic ABI when built on Clang, but when built on GCC, uses an ABI incompatible with both.
GCC's libstdc++ does have a consistent ABI between Clang and GCC, because it always uses GCC's rule, even when built with Clang. But that means it doesn't match C11 _Atomic on Clang, only on GCC.
Making everything worse is that, as of C++23's addition of <stdatomic.h>, expectation of ABI-compatibility between C++ `std::atomic<T>` and C11 `_Atomic(T)` is heightened.
This is something that I think we ought to fix -- but it would require an ABI break. A long time ago, I attempted to bring this up as something the psABI documents should _define_, but didn't manage to push hard enough to get it actually done. My expectation is that if the documents did specify it, they would specify what GCC does. Clang's option is, in principle, nice from a QoI standpoint (it allows more types to be lock-free), but in practice is simply unnecessary complexity.
### Mess 2: libc++ atomic configuration options
OK, so, that's a huge minefield. But for atomic_ref, we can actually ignore all of the above complexity, since we cannot change the layout of the underlying type. Thus: we can simply implement atomic_ref directly on the `__atomic_*` builtins, without going through libc++'s `__cxx_atomic_*` abstraction layer -- leaving fixing that mess for another day. The abstraction layer really doesn't get us anything!
Well, _except_ that libc++ has a _third_ implementation strategy that std::atomic can use. Namely, 21450545d144ed85cb5bc15ab3e15f9a6583af40 added a custom locked impl under `_LIBCPP_ATOMIC_ONLY_USE_BUILTINS`.
IMO, that is effectively superfluous and incompatible with the goal of C and C++ equivalency, and I don't really understand the rationale for adding it in the first place. If someone wants to use locked-atomics in a freestanding implementation, they should simply provide the requisite libatomic libcalls for their environment, end of story.
So, I think we should remove this third option. Or, at least, ignore it when implementing atomic_ref, and do just use the `__atomic_*` builtins directly.
https://github.com/llvm/llvm-project/pull/76647
More information about the libcxx-commits
mailing list