> On 23 Aug 2019, at 10:35, Jonathan Wakely <jwakely....@gmail.com> wrote:
>
> On Fri, 23 Aug 2019 at 08:21, Iain Sandoe <idsan...@googlemail.com> wrote:
>>
>> Hi Jim,
>>
>>> On 23 Aug 2019, at 00:56, Jim Wilson <j...@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
