* gcc vs clang for non-power-2 atomic structures @ 2019-08-22 23:56 Jim Wilson 2019-08-23 7:21 ` Iain Sandoe 0 siblings, 1 reply; 8+ messages in thread From: Jim Wilson @ 2019-08-22 23:56 UTC (permalink / raw) To: GCC Development 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. 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. 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 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-22 23:56 gcc vs clang for non-power-2 atomic structures Jim Wilson @ 2019-08-23 7:21 ` Iain Sandoe 2019-08-23 9:36 ` Jonathan Wakely 0 siblings, 1 reply; 8+ messages in thread From: Iain Sandoe @ 2019-08-23 7:21 UTC (permalink / raw) To: Jim Wilson; +Cc: GCC Development 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-23 7:21 ` Iain Sandoe @ 2019-08-23 9:36 ` Jonathan Wakely 2019-08-23 10:13 ` Iain Sandoe 0 siblings, 1 reply; 8+ messages in thread From: Jonathan Wakely @ 2019-08-23 9:36 UTC (permalink / raw) To: Iain Sandoe; +Cc: Jim Wilson, GCC Development 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 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. > 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-23 9:36 ` Jonathan Wakely @ 2019-08-23 10:13 ` Iain Sandoe 2019-08-23 11:17 ` Jonathan Wakely 2019-08-23 16:14 ` Joseph Myers 0 siblings, 2 replies; 8+ messages in thread From: Iain Sandoe @ 2019-08-23 10:13 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jim Wilson, GCC Development > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-23 10:13 ` Iain Sandoe @ 2019-08-23 11:17 ` Jonathan Wakely 2019-08-23 16:14 ` Joseph Myers 1 sibling, 0 replies; 8+ messages in thread From: Jonathan Wakely @ 2019-08-23 11:17 UTC (permalink / raw) To: Iain Sandoe; +Cc: Jim Wilson, gcc On Fri, 23 Aug 2019, 11:13 Iain Sandoe, <idsandoe@googlemail.com> wrote: > > > > 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? Is that for IA-32 as well, or only x86-64 and x32? I thought the Google group was the right place for IA-32 issues, and that would then get picked up by the x86-64 psABI too. But I don't know. It would be helpful if these things weren't spread around over so many different locations with no actual fixes happening. Just to confuse me further, the wiki at that GitHub repo says to go to gitlab! "All x86-64 psABI changes should be sent to https://gitlab.com/x86-psABIs/x86-64-ABI." It feels like nobody cares about fixing this. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-23 10:13 ` Iain Sandoe 2019-08-23 11:17 ` Jonathan Wakely @ 2019-08-23 16:14 ` Joseph Myers 2019-08-23 18:49 ` Jim Wilson 2019-08-23 18:59 ` Iain Sandoe 1 sibling, 2 replies; 8+ messages in thread From: Joseph Myers @ 2019-08-23 16:14 UTC (permalink / raw) To: Iain Sandoe; +Cc: Jonathan Wakely, Jim Wilson, GCC Development [-- Attachment #1: Type: text/plain, Size: 1877 bytes --] On Fri, 23 Aug 2019, Iain Sandoe wrote: > 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⦠With over 50 target architectures supported in GCC, most of which probably don't have anyone maintaining a psABI for them, you don't get support for new language features that require an ABI without making some reasonable default choice that allows the features to work everywhere and then letting architecture maintainers liaise with ABI maintainers in the case where such exist. (ABIs for atomics have the further tricky issue that there can be multiple choices for how to map the memory model onto a given architecture; see <https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html>. So it's not just a matter of type sizes and alignment.) There *is* a clear GCC bug (bug 65146) in the specific case of _Atomic long long / _Atomic double in structures on 32-bit x86; those need to be forced to 8-byte alignment when atomic as they are outside structures. Size / alignment for _Atomic versions of types whose size isn't (2, 4, 8, 16) bytes is another matter; the GCC default (don't change size / alignment when making atomic) seems perfectly reasonable, but where psABIs specify something we do of course need to follow it (and the choice may be OS-specific, not just processor-specific, where the ABI is defined by the default compiler for some OS). Note: it's likely some front-end code, and stdatomic.h, might have to change to handle the possibility of atomic types being larger than non-atomic, as those end up using type-generic atomic load / store built-in functions, and those certainly expect pointers to arguments of the same size (when one argument is the atomic type and one non-atomic). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-23 16:14 ` Joseph Myers @ 2019-08-23 18:49 ` Jim Wilson 2019-08-23 18:59 ` Iain Sandoe 1 sibling, 0 replies; 8+ messages in thread From: Jim Wilson @ 2019-08-23 18:49 UTC (permalink / raw) To: Joseph Myers; +Cc: Iain Sandoe, Jonathan Wakely, GCC Development I was pointed at https://bugs.llvm.org/show_bug.cgi?id=26462 for the LLVM discussion of this problem. Another issue here is that we should have ABI testing for atomic. For instance, gcc/testsuite/gcc.dg/compat has no atomic testcases. Likewise g++.dg/compat. Jim ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: gcc vs clang for non-power-2 atomic structures 2019-08-23 16:14 ` Joseph Myers 2019-08-23 18:49 ` Jim Wilson @ 2019-08-23 18:59 ` Iain Sandoe 1 sibling, 0 replies; 8+ messages in thread From: Iain Sandoe @ 2019-08-23 18:59 UTC (permalink / raw) To: Joseph Myers; +Cc: Jonathan Wakely, Jim Wilson, GCC Development Hi Joseph, > On 23 Aug 2019, at 17:14, Joseph Myers <joseph@codesourcery.com> wrote: > > On Fri, 23 Aug 2019, Iain Sandoe wrote: > >> 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… > > With over 50 target architectures supported in GCC, most of which probably > don't have anyone maintaining a psABI for them, you don't get support for > new language features that require an ABI without making some reasonable > default choice that allows the features to work everywhere and then > letting architecture maintainers liaise with ABI maintainers in the case > where such exist. yes. That’s perfectly reasonable However, it’s more than a little disappointing that X86, for which I would hope that the psABI _was_ considered supported, remains silent on the issue so long after it arose (I guess the interested parties with $ need to sponsor some work to update it). > (ABIs for atomics have the further tricky issue that there can be multiple > choices for how to map the memory model onto a given architecture; see > <https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html>. So it's not > just a matter of type sizes and alignment.) Indeed, I have tangled a bit with that trying to adapt libatomic to be better behaved on Darwin. > but where psABIs > specify something we do of course need to follow it (and the choice may be > OS-specific, not just processor-specific, where the ABI is defined by the > default compiler for some OS). agreed .. it seems highly likely for X86 as things stand - since there’s a bunch of things already out there with different ABIs baked in. > > Note: it's likely some front-end code, and stdatomic.h, might have to > change to handle the possibility of atomic types being larger than > non-atomic, as those end up using type-generic atomic load / store > built-in functions, and those certainly expect pointers to arguments of > the same size (when one argument is the atomic type and one non-atomic). It seems to me that whatever might be chosen for the definitive psABI / platform (i.e. arch + OS + version) going forward, we will need to support what has been emitted in the past. So a recommendation for suitable FE hooks (and preferably a way to make the C11 atomic match the std::atomic, even if this is “only” a QoI issue), would be worth addressing. thanks Iain > > -- > Joseph S. Myers > joseph@codesourcery.com ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-23 18:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-22 23:56 gcc vs clang for non-power-2 atomic structures Jim Wilson 2019-08-23 7:21 ` Iain Sandoe 2019-08-23 9:36 ` Jonathan Wakely 2019-08-23 10:13 ` Iain Sandoe 2019-08-23 11:17 ` Jonathan Wakely 2019-08-23 16:14 ` Joseph Myers 2019-08-23 18:49 ` Jim Wilson 2019-08-23 18:59 ` Iain Sandoe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).