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

[Iain Sandoe <idsandoe@googlemail.com>]

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.

> 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. 

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.

HTH,
Iain

> The original RISC-V bug report is at
>    https://github.com/riscv/riscv-elf-psabi-doc/pull/112
> There is a pointer to a gist with a larger testcase with RISC-V results.
> 
> Jim