Re: gcc vs clang for non-power-2 atomic structures

[Iain Sandoe <idsandoe@googlemail.com>]

> On 23 Aug 2019, at 10:35, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> 
> On Fri, 23 Aug 2019 at 08:21, Iain Sandoe <idsandoe@googlemail.com> wrote:
>> 
>> Hi Jim,
>> 
>>> On 23 Aug 2019, at 00:56, Jim Wilson <jimw@sifive.com> wrote:
>>> 
>>> We got a change request for the RISC-V psABI to define the atomic
>>> structure size and alignment.  And looking at this, it turned out that
>>> gcc and clang are implementing this differently.  Consider this
>>> testcase
>>> 
>>> rohan:2274$ cat tmp.c
>>> #include <stdio.h>
>>> struct s { int a; int b; int c;};
>>> int
>>> main(void)
>>> {
>>> printf("size=%ld align=%ld\n", sizeof (struct s), _Alignof(struct s));
>>> printf("size=%ld align=%ld\n", sizeof (_Atomic (struct s)),
>>> _Alignof(_Atomic (struct s)));
>>> return 0;
>>> }
>>> rohan:2275$ gcc tmp.c
>>> rohan:2276$ ./a.out
>>> size=12 align=4
>>> size=12 align=4
>>> rohan:2277$ clang tmp.c
>>> rohan:2278$ ./a.out
>>> size=12 align=4
>>> size=16 align=16
>>> rohan:2279$
>>> 
>>> This is with an x86 compiler.
>> 
>> A search for _Atomic in the latest (x86_64) psABI document shows no results.
> 
> See https://groups.google.com/forum/#!topic/ia32-abi/Tlu6Hs-ohPY and
> the various GCC bugs linked to from that thread.
> 
> This is a big can of worms, and GCC needs fixing (but probably not
> until the ABI group decide something).

I agree about the size of the can of worms.
However, there doesn’t seem to be any actual issue filed on:
https://github.com/hjl-tools/x86-psABI

Would it help if someone did?

>>> I get the same result with a RISC-V
>>> compiler.  This is an ABI incompatibility between gcc and clang.  gcc
>>> has code in build_qualified_type in tree.c that sets alignment for
>>> power-of-2 structs to the same size integer alignment, but we don't
>>> change alignment for non-power-of-2 structs.  Clang is padding the
>>> size of non-power-of-2 structs to the next power-of-2 and giving them
>>> that alignment.
>> 
>> If the psABI makes no statement about what _Atomic should do, AFAIK
>> the compiler is free to do something different (for the same entity) from
>> the non-_Atomic version.
> 
> Right, but GCC and Clang should agree. So it needs to be in the psABI.

absolutely, it’s the psABI that’s lacking here - the compilers (as commented
by Richard Smith in a referenced thread) should not be making ABI up…

>> e.g from 6.2.5 of n2176 (C18 draft)
>> 
>>        • 27  Further, there is the _Atomic qualifier. The presence of the _Atomic qualifier designates an atomic type. The size, representation, and alignment of an atomic type need not be the same as those of the corresponding unqualified type. Therefore, this Standard explicitly uses the phrase “atomic, qualified or unqualified type” whenever the atomic version of a type is permitted along with the other qualified versions of a type. The phrase “qualified or unqualified type”, without specific mention of atomic, does not include the atomic types.
>> 
>> So the hole (in both cases) to be plugged is to add specification for _Atomic
>> variants to the psABI….
>> 
>> … of course, it makes sense for the psABI maintainers to discuss with
>> the compiler “vendors” what makes the best choice.
>> 
>> (and it’s probably a significant piece of work to figure out all the interactions
>> with __attribute__ modifiers)
>> 
>>> Unfortunately, I don't know who to contact on the clang side, but we
>>> need to have a discussion here, and we probably need to fix one of the
>>> compilers to match the other one, as we should not have ABI
>>> incompatibilities like this between gcc and clang.
>> 
>> The equivalent of “MAINTAINERS” in the LLVM sources for backends is
>> “llvm/CODE_OWNERS.TXT” which says that Alex Bradbury is the code
>> owner for the RISC-V backend.
> 
> Tim Northover and JF Bastien at Apple should probably be involved too.
> 
> IMO GCC is broken, and the longer we take to fix it the more painful
> it will be for users as there will be more code affected by the
> change.

I suspect that (even if this is not a solution chosen elsewhere), for Darwin,
there will have to be a target hook to control this since I can’t change the
ABI retrospectively for the systems already released.  That is, GCC is emitting
broken code on those platforms anyway (since the platform ABI is determined
by what clang/llvm produces).

Iain