GCC/clang C11 _Atomic incompatibility

[jyknight]

Bugzilla Link	26462
Version	unspecified
OS	Linux
CC	@majnemer,@hfinkel,@kbeyls,@zygoloid,@rjmccall,@lenary,@tstellar,@jwakely

Extended Description

Clang and GCC have incompatible ABIs for _Atomic, on non-power-of-2-sized types.

Simple demonstration of the difference:

struct A3 { char val[3]; };
_Atomic struct A3 a3;
// GCC:
_Static_assert(sizeof(a3) == 3, "");
_Static_assert(_Alignof(a3) == 1, "");
// Clang:
_Static_assert(sizeof(a3) == 4, "");
_Static_assert(_Alignof(a3) == 4, "");

GCC's logic for _Atomic is: For types which have a size of exactly 1, 2, 4, 8, or 16 bytes, increase the alignment to be at least the size. Never change the size of the type.

libstdc++'s std::atomic uses the same logic as GCC, but it's implemented inline in the header, as GCC doesn't support C11 atomics in C++ mode. Thus, libstdc++ under clang also uses GCC's rule.

Clang has the following rule: if the size of a type is less than a target-specific variable MaxAtomicPromoteWidth (0, 4, 8, or 16 bytes on current targets), round the size up to the next power of two, and SET the alignment to the size.

libc++'s std::atomic uses clang's C11 atomics support (which clang supports as an extension in C++ mode), and thus gets the same behavior...but only when built with clang. When libc++ is built with GCC, it uses an alternative implementation which doesn't ever increase the alignment/size.

So, the current situation:

• C11 _Atomic is incompatible between Clang and GCC.
• libstdc++'s std::atomic is compatible between Clang and GCC.
• libc++'s std::atomic is incompatible between Clang and GCC.

Furthermore, I believe C11 and C++ atomics are intended to be compatible with eachother. And that's not true with clang and libstdc++, nor with gcc and libc++.

[jyknight]

So, what should be done? AFAICT, atomics aren't mentioned in the sysv abi docs at all, so we have no official guidance.

Clang's behavior of rounding up and adding padding is theoretically nice, since it allows efficient atomics for sizeof(X) == 3, but does add a bunch of complexity to the compiler related to padding when sizeof(_Atomic X) != sizeof(X).

GCC's implementation is nice and simple, but it has the downside of sizeof(X) == 3 objects must use _atomic* libcalls for all operations, since there's no inline atomic support for size-3, align-1 objects.

I seriously question whether any users actually care about efficient atomic ops on odd-sized objects. This seems like a really edgey edge-case. If I could go back in time, I'd suggest that clang not have implemented the size-modification at all.

But, now...

I would propose that the best option now is to change clang's ABI (affecting: C11 _Atomic and libc++ std::atomic on strangely-sized types) to match GCC's, and add a warning where the resulting size/alignment turns out different.

[zygoloid]

The C and C++ committees intended and expected that _Atomic T / std::atomic<T> would be padded out to an atomic size when possible. There is explicit allowance for this in both C and C++ as a result, and Clang follows that.

The only semi-official documentation I can find on GCC's intent here is this:

https://gcc.gnu.org/wiki/Atomic/GCCMM/UnalignedPolicy

(Note that this matches what the standards committees appear to have desired, and what Clang implements.) However, for whatever reason, they don't follow that document; see the explicit acknowledgement of that in https://gcc.gnu.org/PR65146 .

It seems like Clang's choice is strictly better, and ABI compatibility with previous versions of Clang is an important consideration here. In the first instance, I think we should ask the GCC folks why they chose not to expand the size of _Atomic to an atomic size, and whether they consider this to be a bug and might be interested in fixing it. If the GCC folks aren't willing or able to budge, we'll need to decide whether we care more about GCC compatibility or compatibility with previous versions of Clang / better _Atomic performance.

Separately, for libc++, the ABI established when compiling with Clang seems to take priority (regardless of which option we think is best; libc++ is used with Clang vastly more than with GCC), and the GCC-compatibility support should just be fixed to pick an appropriate size and alignment for the atomic storage.

[jwakely]

Furthermore, I believe C11 and C++ atomics are intended to be compatible
with eachother.

That was originally the aim of the people working on the atomics proposals for C++11 and C11, but it didn't end up that way. It's purely QoI whether there is any compatibility between _Atomic T and std::atomic.

[jyknight]

It's purely QoI whether there is any compatibility between _Atomic T and std::atomic.

Oh well, that's unfortunate. We can ignore that part of the problem then, I guess (except inasmuch as that if we change anything, it would be nice to have and/or preserve that QoI.)

Separately, for libc++, the ABI established when compiling with Clang seems to take priority

I agree, the use of libc++ with gcc is nearly non-existent, and is the least important issue.

and the GCC-compatibility support should just be fixed to pick an appropriate size and alignment for the atomic storage.

However, making the libc++ GCC-compatibility support work the same as libc++ on LLVM is rather tricky, since the MaxAtomicPromoteWidth value in clang, which is required to do the correct calculation, is target-specific. So, if libc++ on GCC is to be compatible with clang's current algorithm, it would have to copy that value into the header guarded with proper conditionals for each of the known targets.

Unfortunately, I've also just realized that this implies there are GCC/Clang ABI differences even for power-of-2 sized types.

E.g., on powerpc64, clang sets MaxAtomicPromoteWidth to 64. Thus, the following:

struct A16 { char val[16]; };
_Atomic struct A16 a16;
// GCC:
_Static_assert(_Alignof(a16) == 16, "");
// Clang:
_Static_assert(_Alignof(a16) == 1, "");

ACTUALLY, even disregarding the ABI incompatibility problem, the way clang currently limits the promotion size is quite unfortunate, since it has encoded the current maximum supported atomic size of a CPU architecture into the ABI.

That somewhat nullifies the goal of being able to use libatomic for forwards-compatible atomic support, which can grow more efficient automatically as new processors add larger atomic instructions.

For example, PPC actually has 128-bit atomic instructions, now ("lqarx/stqcx", new in v2.07 of the ISA). The strategy for allowing use of them is supposed to be:

1. Rebuild libatomic to (dynamically) use them when appropriate, instead of locks.
2. Update compiler to emit them for code targeting a late-enough microarch.

And, magic, no ABI incompatibilities, all code interoperates, but you have efficient larger atomics! But that won't work out properly, if the alignment isn't sufficient.

Always increasing the alignment of atomics up to 128 bit would've been a better plan...

[rjmccall]

We talked about that issue a few years on one of the mailing lists (I'm pretty sure cfe-dev was CC'ed). Very high alignment requirements have very high padding costs (on both sides of the data) and frequently run into poor support from the rest of the toolchain and runtime, most importantly malloc. Meanwhile, if locking is acceptable on old systems, it's probably acceptable on new systems, too; and it's easy for programmers to opt in to the new atomics by explicitly making their memory over-aligned.

[rjmccall]

Anyway, while I prefer the Clang rule, the GCC rule is defensible, as are any number of other rules. The important point, however, is that having this discussion is not the right approach to solving this problem. The layout of _Atomic(T) is ABI. ABI rules are not generally determined by compiler implementors making things up as they go along, or at least they shouldn't be. The Darwin ABI for _Atomic is the rule implemented in Clang, which we actually did think about carefully when we adopted it. Other platforms need to make their own call, and it probably shouldn't just be "whatever's implemented in GCC", especially on other platforms where GCC is not the system compiler.

[jyknight]

Very high alignment requirements have very high padding costs (on both sides of the data)
and frequently run into poor support from the rest of the toolchain and runtime, most
importantly malloc

I wouldn't say that 128bit/16byte alignment is very high. But as for proper support from 'malloc', that is a very good point!

That does seem to indicate that GCC's implementation of _Atomic and std::atomic is potentially broken on many 32-bit platforms, and probably some 64-bit platforms as well. (That is: it seems common, although not universal, for 32-bit platforms to provide 8-byte alignment from malloc, and 64-bit platforms to provide 16-byte alignment).

It's possible that the brokenness is not a problem in practice, since the hardware probably doesn't actually require 16-byte alignment for anything.

Even with that in mind, clang's maximums are unfortunately low, being often lower than the alignment of long long.

The important point, however, is that having
this discussion is not the right approach to solving this problem.

Well, hopefully this bug is a start to solving the problem: recognizing there is a problem and discussing some possible solutions.

The
layout of _Atomic(T) is ABI. ABI rules are not generally determined by
compiler implementors making things up as they go along, or at least they
shouldn't be.

Indeed, I found it quite surprising and unfortunate that the SysV ABI documents have not been updated to cover the ABI impact of _Atomic at all; it would have been real nice for that to have happened before it was implemented differently, separately, by GCC and Clang. But that didn't happen, so now there's a problem.

It does somewhat appear as if all compiler vendors have in fact implemented a new ABI by "making things up as they go along".

The Darwin ABI for _Atomic is the rule implemented in Clang,
which we actually did think about carefully when we adopted it.

Good to hear; I wasn't involved in the llvm/clang community back then, so I have no context of what happened. However, at the moment, the Darwin ABI for x86-64 seems to be defined by a page [1] which simply refers to the SysV ABI document for all details. As mentioned, the sysv abi document doesn't define an ABI for _Atomic. The Darwin ABIs for other architectures also don't mention _Atomic at all. So I'm afraid Darwin hardly seems any better off here. Or do you have some other documentation?

[1] https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/LowLevelABI/140-x86-64_Function_Calling_Conventions/x86_64.html

[llvmbot]

Proposed patch for the _Atomic issue
The following should fix the _Atomic incompatibility issue; the fix is limited to Linux platforms and should not affect others.

This is the first time I'm trying to add tests and I can't find a log file giving pass/fail results to verify they actually run, so please advise if I'm doing it wrong.

[zygoloid]

The following should fix the _Atomic incompatibility issue;

Please read the above discussion and particularly comment#6. The thing to do is first raise this on the appropriate ABI lists and get the platform ABI documents updated to describe the _Atomic ABI. Then propose patches to whichever compiler(s) don't follow the rule.

Also note that if the result is that clang needs to change its _Atomic, we'll also need to decide if this means libc++ takes an ABI break or if its std::atomic loses its ABI compatibility guarantee with _Atomic.

[lenary]

This issue came up when implementing the RISC-V psABI in Clang. That ABI is in flux at the moment, and so we have the opportunity to fully define these details.

In talking with the GCC implementors (the RISC-V psABI is currently based on the GCC implementation), they have acknowledged that this incompatibility is an issue, and raised it on the GCC Mailing list.

The GCC Mailing List thread is here: https://gcc.gnu.org/ml/gcc/2019-08/msg00191.html

[rjmccall]

That's great. It sounds like they're not planning to break ABI for existing targets --- which, as I mentioned, is reasonable enough --- so maybe we should go ahead and add a target hook to control behavior.

Has MSVC implemented _Atomic? It still looks unsupported on godbolt.

[lenary]

That's great. It sounds like they're not planning to break ABI for existing
targets --- which, as I mentioned, is reasonable enough

My understanding is actually contrary to this. Some people do think that clang's approach is better, as it requires less locking, but overall I think they want to raise the issue with the x86 psABI maintainers, and have them decide, and then implementations can be corrected based off that.

They do recognise that this atomic issue is one of their incompatibilities with Darwin, and concede that at least on Darwin they are wrong, but again, there isn't enough documentation and they want the x86 psABI maintainers to jump first.

As it is, that discussion has fizzled out, so I guess the issue has been dropped again.

[rjmccall]

That's great. It sounds like they're not planning to break ABI for existing
targets --- which, as I mentioned, is reasonable enough

My understanding is actually contrary to this. Some people do think that
clang's approach is better, as it requires less locking, but overall I think
they want to raise the issue with the x86 psABI maintainers, and have them
decide, and then implementations can be corrected based off that.

They do recognise that this atomic issue is one of their incompatibilities
with Darwin, and concede that at least on Darwin they are wrong, but again,
there isn't enough documentation and they want the x86 psABI maintainers to
jump first.

As it is, that discussion has fizzled out, so I guess the issue has been
dropped again.

I'd be happy to lean on the psABI maintainers with whatever weight I can find, but I'm not sure who I should be talking to. Is that these people? https://gitlab.com/x86-psABIs/x86-64-ABI

[jwakely]

That's for x86_64, but I think it needs to start with x86 and then propagate to x86_64. The IA-32 psABI lives at
https://github.com/hjl-tools/x86-psABI/tree/hjl/x86/master and I already tried to raise this at https://groups.google.com/forum/#!topic/ia32-abi/Tlu6Hs-ohPY

[rjmccall]

That's for x86_64, but I think it needs to start with x86 and then propagate
to x86_64. The IA-32 psABI lives at
https://github.com/hjl-tools/x86-psABI/tree/hjl/x86/master and I already
tried to raise this at
https://groups.google.com/forum/#!topic/ia32-abi/Tlu6Hs-ohPY

Thanks. I've chimed in; it's in moderation for now.