My r12-2975-g32c3a75390623a0470df52af13f78baddd562981 change removed removal of zero width unnamed bitfields from the C++ FE, the C FE has never been removing those (the removal was certainly premature, as the newly added traits need to see the zero width unnamed bitfields). This unfortunately can affect ABI on several targets. From quick grep I think this can't affect alpha, arc, avr, bfin, bpf, c6x, cr16, cris, csky, epiphany, fr30, frv, ft32, gcn, h8300, lm32, m32c, m32r, m68k, mcore, microblaze, mmix, mn10300, moxie, msp430, nds32, nios2, nvptx, or1k, pa, pdp11, pru, rl78, rx, stormy16, tilegx, tilepro, v850, vax, visium, xtensa are all safe, they either don't mention TYPE_FIELDS in their backends or use it either in va_list or in structure layout code only (structure layout happens before the zero width bitfields are removed by the FE). TYPE_MODE should be ok too, it is just passing of arguments by value and returning by value that could be problematic on aarch64, arm, i386, ia64, iq2000, mips, riscv, rs6000, s390, sh and sparc I think sh is ok, it ignores integer_zerop (DECL_SIZE (field)). riscv apparently has old (< GCC 10) and new (>= GCC 10) ABI and the old one didn't ignore them and made C and C++ ABI incompatible, while the new one does ignore them. So, my patch probably broke the "ABI for flattened struct with zero-length bit-fields changed in GCC 10" warning (why isn't it guarded with -Wpsabi?), perhaps we should mark the C++ zero width bitfields with some attribute or flag so that backends could emit correct -Wpsabi warnings? x86_64 seems to be affected: struct S { float a; int : 0; float b; }; __attribute__((noipa)) struct S foo (struct S x) { return x; } void bar (void) { struct S s = { 0.0f, 0.0f }; foo (s); } With GCC 10 and C we emit: xorl %edi, %edi jmp foo With C++ we emit: pxor %xmm0, %xmm0 jmp _Z3foo1S with current trunk xorl %edi, %edi jmp _Z3foo1S
clang and clang++ on x86_64 ignore the zero width bitfields, so work similarly to C++ in GCC < 12.
Perhaps we can use DECL_FIELD_ABI_IGNORED flag for the zero width bitfields from C++ FE, right now that flag is only used on aggregate FIELD_DECLs.
PowerPC seems to be (tried powerpc64le) affected as well on the same testcase, addi 2,2,.TOC.-.LCF0@l .localentry _Z3barv,.-_Z3barv mflr 0 - li 9,0 - sldi 9,9,32 - mtvsrd 1,9 + li 3,0 std 0,16(1) stdu 1,-32(1) .cfi_def_cfa_offset 32 .cfi_offset 65, 16 - xscvspdpn 1,1 - fmr 2,1 bl _Z3foo1S nop addi 1,1,32 seems to be the difference from C++ before my change to C++ after my change and the after my change matches what C did before and does now as well. So like for x86-64 psABI, the question is what the psABI says and what we should use and either way we need -Wpsabi warning for the case we'll change ABI for.
Thanks for raising this. Just to summarise what I said on irc: For aarch64 we'll have to decide between: (a) keeping everything as it was for GCC 11, including the difference between C and C++ (b) keep the old C++ behaviour and make C match it (c) keep the old C behaviour and make C++ match it (i.e. what happens on trunk now) None of these are particularly appealing. My understanding is that (2) is what the psABI says, but we also have clang compatibility to consider. We'll need to discuss this internally a bit. Obviously something needs to be done in time for GCC 12, but due to other pressures, I can't promise anything in time for end of stage 1.
"structure layout happens before the zero width bitfields are removed by the FE". So, what can break still, then? - li 9,0 - sldi 9,9,32 - mtvsrd 1,9 + li 3,0 std 0,16(1) stdu 1,-32(1) .cfi_def_cfa_offset 32 .cfi_offset 65, 16 - xscvspdpn 1,1 - fmr 2,1 1) The old code was very inefficient, didn't have constants optimised; 2) The old code passed "s" in an FPR, the new one uses a GPR? Can you also show what changed in "foo"?
(In reply to Segher Boessenkool from comment #5) > "structure layout happens before the zero width bitfields are removed by the > FE". > > So, what can break still, then? Homogeneous aggregates can break. Before there wasn't one, but with the :0 removed there is. > - li 9,0 > - sldi 9,9,32 > - mtvsrd 1,9 > + li 3,0 > std 0,16(1) > stdu 1,-32(1) > .cfi_def_cfa_offset 32 > .cfi_offset 65, 16 > - xscvspdpn 1,1 > - fmr 2,1 So this shows the backwards change?
So my gut feeling is that zero-width bitfields are more likely to be present in C code and thus not breaking the C ABI might be preferred. Also any ABI incompatibility between C and C++ is bad so we do want to change either the C or the C++ FEs here.
The only thing the x86-64 psABI says about this case is "'Unnamed bit-fields' types do not affect the alignment of a structure or union." . (zero-width bit-fields are _always_ unnamed) But the x86-64 psABI was written with the assumption in mind that zero-width bitfields influence the alignment of the following field (i.e. that they break the current bit bucket for two neighboring bitfield members). I.e. exactly what GCCs stor-layout.c code always did for these when not in microsoft compat mode. In the example in question this align-next-bitfield item doesn't come into play. But it does show that zero-width bitfields were designed and allowed as extension for a purpose. So it makes sense for instance on platforms that have a concept of homogenous aggregates that zero-width bit-fields disable those, without otherwise changing memory layout. So, I think, not removing those members from the FE makes sense, they contain crucial information. Unfortunately that means that they need to be dealt with in code dealing with layout (correct) or argument/return-value passing (seemingly fishy right now for some platforms?). Also, all else being equal I think the C language defines the de-facto psABI, so any difference between C and C++ should be resolved towards C.
(In reply to Michael Matz from comment #8) > So, I think, not removing those members from the FE makes sense, they contain > crucial information. Unfortunately that means that they need to be dealt > with > in code dealing with layout (correct) or argument/return-value passing > (seemingly > fishy right now for some platforms?). > > Also, all else being equal I think the C language defines the de-facto > psABI, so any difference between C and C++ should be resolved towards C. Yeah. In the case of Power homogeneous arguments, removing the bitfield from { float a; int:0; float b; } is fine to do *later*, when the backend already knows it is an homogeneous argument, when all decisions have been made and the results recorded. It would be good if this was documented somewhere, where that spot is?
Sorry to be late jumping in. Previously zero bitfields were spuriously removed, now they're being left in place and matching C. That's very good. As Jakub shows, the biggest problem is with homogeneous aggregates of floating-point and vector types in the ELFv2 ABI used for powerpc64le today. Before Jakub's change, we would (wrongly) treat the "struct S" as a homogeneous aggregate and pass the arguments in floating-point registers. With Jakub's change, we (correctly) pass the arguments in GPRs and/or memory. Otherwise I think our ABIs are unaffected. We'll need to add a Wpsabi message in the case where we used to recognize a homogeneous aggregate but no longer do. We ought to be able to add that easily in rs6000_discover_homogeneous_aggregate (rs6000-call.c).
Actually, on further review, I guess we have additional concerns. Unnamed bitfields also have the effect of updating alignment of the subsequent field of a structure. "The types of unnamed bit fields have no effect on the alignment of a structure or union. However, the offsets of an individual bit field's member must comply with the alignment rules. An unnamed bit field of zero width causes sufficient padding (possibly none) to be inserted for the next member, or the end of the structure if there are no more nonzero width members, to have an offset from the start of the structure that is a multiple of the size of the declared type of the zero-width member." That's from ELFv2, but I imagine the same language exists in ELFv1, as I don't believe this was meant to be a change from the previous ABI. An example from the ABI to test is: struct { char c; int : 0; char d; short : 9; char e; }; I wouuld expect that prior to Jakub's change, we would not see three bytes of pad between c and d, but with his change, we would. We can probably warn for this as well, but might be a little trickier. This is more likely to come up than the homogeneous aggregate case, I guess.
Never mind my last comment. Segher pointed out that structure layout is done early enough that this isn't a problem. I verified this using g++ from GCC 11 and GCC 12 to show that we get correct offsets for the previous example in both cases.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:e902136b310ee17d4b49eb42d9d5e487d5dcf4a1 commit r12-3324-ge902136b310ee17d4b49eb42d9d5e487d5dcf4a1 Author: Jakub Jelinek <jakub@redhat.com> Date: Fri Sep 3 09:46:32 2021 +0200 c++, abi: Set DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on C++ zero width bitfields [PR102024] The removal of remove_zero_width_bitfields function and its call from C++ FE layout_class_type (which I've done in the P0466R5 layout-compatible helper intrinsics patch, so that the FE can actually determine what is and isn't layout-compatible according to the spec) unfortunately changed the ABI on various platforms. The C FE has been keeping zero-width bitfields in the types, while the C++ FE has been removing them after structure layout, so in various cases when passing such structures in registers we had different ABI between C and C++. While both the C and C++ FE had some code to remove zero width bitfields after structure layout, in both FEs it was buggy and didn't really remove any. In the C FE that code has been removed later on, while in the C++ FE for GCC 4.5 in PR42217 it has been actually fixed, so the C++ FE started to remove those bitfields. The following patch doesn't change anything ABI-wise, but allows the targets to decide what to do, emit -Wpsabi warnings etc. Non-C zero width bitfields will be seen by the backends as normal zero width bitfields, C++ zero width bitfields that used to be previously removed will have DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD flag set. I've reused the DECL_FIELD_ABI_IGNORED flag which is only used on non-bitfield FIELD_DECLs right now, but the macros now check DECL_BIT_FIELD flag. Each backend can then decide what it wants, whether it wants to keep different ABI between C and C++ as in GCC 11 and older (i.e. incompatible with G++ <= 4.4, compatible with G++ 4.5 .. 11), for that it would ignore for the aggregate passing/returning decisions all DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD FIELD_DECLs), whether it wants to never ignore zero width bitfields (no changes needed for that case, except perhaps -Wpsabi warning should be added and for that DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD can be tested), or whether it wants to always ignore zero width bitfields (I think e.g. riscv in GCC 10+ does that). All this patch does is set the flag which the backends can then use. 2021-09-03 Jakub Jelinek <jakub@redhat.com> PR target/102024 gcc/ * tree.h (DECL_FIELD_ABI_IGNORED): Changed into rvalue only macro that is false if DECL_BIT_FIELD. (SET_DECL_FIELD_ABI_IGNORED, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD, SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD): Define. * tree-streamer-out.c (pack_ts_decl_common_value_fields): For DECL_BIT_FIELD stream DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead of DECL_FIELD_ABI_IGNORED. * tree-streamer-in.c (unpack_ts_decl_common_value_fields): Use SET_DECL_FIELD_ABI_IGNORED instead of writing to DECL_FIELD_ABI_IGNORED and for DECL_BIT_FIELD use SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead. * lto-streamer-out.c (hash_tree): For DECL_BIT_FIELD hash DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead of DECL_FIELD_ABI_IGNORED. gcc/cp/ * class.c (build_base_field): Use SET_DECL_FIELD_ABI_IGNORED instead of writing to DECL_FIELD_ABI_IGNORED. (layout_class_type): Likewise. In the place where zero-width bitfields used to be removed, use SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on those fields instead. gcc/lto/ * lto-common.c (compare_tree_sccs_1): Also compare DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD values.
The generic support is now committed, each backend can decide on what to do using those flags. That decision should be guided by psABI if it is clear on those matters, or otherwise ideally the psABIs should be clarified.
Jakub, could you add a note with a section ID in https://gcc.gnu.org/gcc-12/changes.html as was done for the similar case in GCC 10? This allowed us to specify a URL in the informational diagnostic, like so: const char *url = CHANGES_ROOT_URL "gcc-10/changes.html#empty_base"; inform (input_location, "parameter passing for argument of type %qT " "when C++17 is enabled changed to match C++14 " "%{in GCC 10.1%}", type, url);
The master branch has been updated by William Schmidt <wschmidt@gcc.gnu.org>: https://gcc.gnu.org/g:16e3d6b8b2b5168ebc773833f0e7ccf2191932c1 commit r12-3843-g16e3d6b8b2b5168ebc773833f0e7ccf2191932c1 Author: Bill Schmidt <wschmidt@linux.ibm.com> Date: Thu Sep 23 07:35:42 2021 -0500 rs6000: Add psabi diagnostic for C++ zero-width bit field ABI change Previously zero-width bit fields were removed from structs, so that otherwise homogeneous aggregates were treated as such and passed in FPRs and VSRs. This was incorrect behavior per the ELFv2 ABI. Now that these fields are no longer being removed, we generate the correct parameter passing code. Alert the unwary user in the rare cases where this behavior changes. 2021-09-23 Bill Schmidt <wschmidt@linux.ibm.com> gcc/ PR target/102024 * config/rs6000/rs6000-call.c (rs6000_aggregate_candidate): Detect zero-width bit fields and return indicator. (rs6000_discover_homogeneous_aggregate): Diagnose when the presence of a zero-width bit field changes parameter passing in GCC 12. gcc/testsuite/ PR target/102024 * g++.target/powerpc/pr102024.C: New.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:3159da6c46568a7c600f78fb3a3b76e2ea4bf4cc commit r12-6418-g3159da6c46568a7c600f78fb3a3b76e2ea4bf4cc Author: Jakub Jelinek <jakub@redhat.com> Date: Mon Jan 10 17:43:23 2022 +0100 x86_64: Ignore zero width bitfields in ABI and issue -Wpsabi warning about C zero width bitfield ABI changes [PR102024] For zero-width bitfields current GCC classify_argument does: if (DECL_BIT_FIELD (field)) { for (i = (int_bit_position (field) + (bit_offset % 64)) / 8 / 8; i < ((int_bit_position (field) + (bit_offset % 64)) + tree_to_shwi (DECL_SIZE (field)) + 63) / 8 / 8; i++) classes[i] = merge_classes (X86_64_INTEGER_CLASS, classes[i]); } which I think means that if the zero-width bitfields are at bit-positions (in the toplevel aggregate) which are multiples of 64 bits doesn't do anything, (int_bit_position (field) + (bit_offset % 64)) / 64 and (int_bit_position (field) + (bit_offset % 64) + 63) / 64 should be equal. But for zero-width bitfields at other bit positions it will call merge_classes once. Now, the typical case is that the zero width bitfield is surrounded by some bitfields and in that case, it doesn't change anything, but it can be sandwitched in between floats too as the testcases show. In C we had this behavior, in C++ previously the FE was removing the zero-width bitfields and therefore they were ignored. LLVM and ICC seems to ignore those bitfields both in C and C++ (== passing struct S in SSE register rather than in GPR). The x86-64 psABI has been recently clarified by https://gitlab.com/x86-psABIs/x86-64-ABI/-/commit/1aa4398d26c250b252a0c4a0f777216c9a6789ec that zero width bitfield should be always ignored. This patch implements that and emits a warning for C for cases where the ABI changed from GCC 11. 2022-01-10 Jakub Jelinek <jakub@redhat.com> PR target/102024 * config/i386/i386.c (classify_argument): Add zero_width_bitfields argument, when seeing DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD bitfields, always ignore them, when seeing other zero sized bitfields, either set zero_width_bitfields to 1 and ignore it or if equal to 2 process it. Pass it to recursive calls. Add wrapper with old arguments and diagnose ABI differences for C structures with zero width bitfields. Formatting fixes. * gcc.target/i386/pr102024.c: New test. * g++.target/i386/pr102024.C: New test.
> TYPE_MODE should be ok too, it is just passing of arguments by value and > returning by value that could be problematic on > aarch64, arm, i386, ia64, iq2000, mips, riscv, rs6000, s390, sh and sparc > I think sh is ok, it ignores integer_zerop (DECL_SIZE (field)). On SPARC, the 32-bit ABI passes structures by reference so is not concerned while the 64-bit ABI does the same as SH: /* Walk the real fields, but skip those with no size or a zero size. ??? Fields with variable offset are handled as having zero offset. */ for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) if (TREE_CODE (field) == FIELD_DECL) { if (!DECL_SIZE (field) || integer_zerop (DECL_SIZE (field))) continue; so is not affected either.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:568377743e22c1377d0aaa1ac9113da3ff1b6bd4 commit r12-7798-g568377743e22c1377d0aaa1ac9113da3ff1b6bd4 Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Mar 24 12:25:15 2022 +0100 testsuite: Add compat.exp testcase for most common zero width bitfld ABI passing [PR102024] On Tue, Mar 22, 2022 at 05:51:58PM +0100, Jakub Jelinek via Gcc wrote: > I guess it would be nice to include the testcases we are talking about, > like { float x; int : 0; float y; } and { float x; int : 0; } and > { int : 0; float x; } into compat.exp testsuite so that we see ABI > differences in compat testing. Here is a patch that does that. It uses the struct-layout-1* framework, but isn't generated because we don't want in this case pseudo-random structure layouts, but particular ones we know cause or could cause problems on some targets. If other problematic cases are discovered, we can add further ones. Tested on x86_64-linux with: make check-gcc check-g++ RUNTESTFLAGS='ALT_CC_UNDER_TEST=gcc ALT_CXX_UNDER_TEST=g++ compat.exp=pr102*' and with make check-gcc check-g++ RUNTESTFLAGS='compat.exp=pr102*' The former as expected has: FAIL: gcc.dg/compat/pr102024 c_compat_x_tst.o-c_compat_y_alt.o execute FAIL: gcc.dg/compat/pr102024 c_compat_x_alt.o-c_compat_y_tst.o execute fails because on x86_64 we've changed the C ABI but kept the C++ ABI here. E.g. on rs6000 it should be the g++.dg such tests to fail (all assuming the alt gcc/g++ is GCC 4.5 through 11). 2022-03-24 Jakub Jelinek <jakub@redhat.com> PR target/102024 * gcc.dg/compat/pr102024_main.c: New test. * gcc.dg/compat/pr102024_test.h: New test. * gcc.dg/compat/pr102024_x.c: New test. * gcc.dg/compat/pr102024_y.c: New test. * g++.dg/compat/pr102024_main.C: New test. * g++.dg/compat/pr102024_test.h: New test. * g++.dg/compat/pr102024_x.C: New test. * g++.dg/compat/pr102024_y.C: New test.
The master branch has been updated by Richard Earnshaw <rearnsha@gcc.gnu.org>: https://gcc.gnu.org/g:3032df28f2a1cc6514571558b76d9b80373b19c6 commit r12-7895-g3032df28f2a1cc6514571558b76d9b80373b19c6 Author: Richard Earnshaw <rearnsha@arm.com> Date: Tue Mar 29 10:24:49 2022 +0100 arm: correctly handle zero-sized bit-fields in AAPCS [PR102024] On arm the AAPCS states that an HFA is determined by the 'shape' of the object after layout has been completed, so anything that adds no members and does not cause the layout to be modified should be ignored for the purposes of determining which registers are used for parameter passing. A zero-sized bit-field falls into this category. This was not handled correctly for C structs and in G++-11 only handled correctly because such fields were eliminated early by the front end. gcc/ChangeLog: PR target/102024 * config/arm/arm.cc (aapcs_vfp_sub_candidate): Handle zero-sized bit-fields. Detect cases where a warning may be needed. (aapcs_vfp_is_call_or_return_candidate): Emit a note if a zero-sized bit-field has caused parameter passing to change. gcc/testsuite/ChangeLog: PR target/102024 * gcc.target/arm/aapcs/vfp26.c: New test.
The master branch has been updated by Richard Earnshaw <rearnsha@gcc.gnu.org>: https://gcc.gnu.org/g:b243ad1afb7f06ef4ab7649600d900b09b9c6b52 commit r12-7896-gb243ad1afb7f06ef4ab7649600d900b09b9c6b52 Author: Richard Earnshaw <rearnsha@arm.com> Date: Tue Mar 29 16:07:09 2022 +0100 aarch64: correctly handle zero-sized bit-fields in AAPCS64 [PR102024] On aarch64 the AAPCS64 states that an HFA is determined by the 'shape' of the object after layout has been completed, so anything that adds no members and does not cause the layout to be modified should be ignored for the purposes of determining which registers are used for parameter passing. A zero-sized bit-field falls into this category. This was not handled correctly for C structs and in G++-11 only handled correctly because such fields were eliminated early by the front end. gcc/ChangeLog: PR target/102024 * config/aarch64/aarch64.cc (aapcs_vfp_sub_candidate): Handle zero-sized bit-fields. Detect cases where a warning may be needed. (aarch64_vfp_is_call_or_return_candidate): Emit a note if a zero-sized bit-field has caused parameter passing to change. gcc/testsuite/ChangeLog: * gcc.target/aarch64/aapcs64/test_28.c: New test.
So, to sum up: aarch64, arm, x86-64 and riscv (last one since GCC 10 already) do ignore zero width bit-fields in argument/return value passing decisions and so have a C ABI incompatibility from earlier GCC versions. There is -Wpsabi diagnostics on affected argument/return value passing. powerpc64le and s390x do not ignore them, so have a C++ ABI incompatibility from earlier GCC versions. There is -Wpsabi diagnostics on affected argument/return value passing. alpha, arc, avr, bfin, bpf, c6x, cr16, cris, csky, epiphany, fr30, frv, ft32, gcn, h8300, i386 (32-bit), lm32, m32c, m32r, m68k, mcore, microblaze, mmix, mn10300, moxie, msp430, nds32, nios2, nvptx, or1k, pa, pdp11, powerpc (except for powerpc64le), pru, rl78, rx, sh, sparc, stormy16, tilegx, tilepro, v850, vax, visium and xtensa are not affected. ia64 and iq2000 are unknown. mips seems to be affected but nothing has been done about it, which means that right now if it is affected, it is not ignoring the zero width bit-fields without -Wpsabi diagnostics. Except for the not affected list and maybe unknown list, there was also an ABI change between G++ 4.4 and earlier and 4.5 and later.
As for mips, https://techpubs.jurassic.nl/manuals/0640/developer/Mpro_n32_ABI/sgi_html/ch02.html says e.g.: "Regardless of the struct field structure, it is treated as a sequence of 64-bit chunks. If a chunk consists solely of a double float field (but not a double, which is part of a union), it is passed in a floating point register. Any other chunk is passed in an integer register." but it is ambiguous if in cases like: struct A { double a; int : 0; double b; }; struct B { double a; int : 0; }; struct C { int : 0; double a; }; those zero width bit-fields are considered part of any chunk and if so, which one (as they are zero size and on the chunk boundaries). The return value rule seems less ambigous: "A struct with only one or two floating point fields is returned in $f0 (and $f2 if necessary). This is a generalization of the Fortran COMPLEX case." my reading of that is that zero width bitfields shouldn't be ignored in that case, because if they are present, the struct doesn't contain only one or two floating point fields, it contains at least the zero width bitfield as well. CCing MIPS maintainers on this (and also LoongArch, while there is no ABI incompatibility for a new port, it is perhaps something to discuss and decide too).
(In reply to Jakub Jelinek from comment #23) > CCing MIPS maintainers on this (and also LoongArch, while there is no ABI > incompatibility for a new port, it is perhaps something to discuss and > decide too). Not a MIPS maintainer, and none of my targets are affected. While it seems likely I was just one the next line, there's a slight chance you meant to add someone else, which would be important and a valid reason for this long sentence and I think I'll stay CC:ed. :)
Sorry, apparently I must have misread the mmix below mips as being mips too.
Reading the mips_function_arg code in detail, it seems it has been ignoring the zero width bitfields because it only checks for 64-bit aligned 64-bit double fields and if they appear, passes that part of struct in FPRs, otherwise in GPRs. So there is likely no change in that case from older releases (of course it is a question what the psABI wants). mips_fpr_return_fields seems to be affected, but the psABI wording (at least my reading thereof) seems to be that int:0; should make it return 0 like it now does (but didn't for C++ in 4.5 to 11). Thus maybe only -Wpsabi diagnostics for mips_fpr_return_fields is missing. Let me close this as a P1 for now then.
(In reply to Jakub Jelinek from comment #23) > struct A { double a; int : 0; double b; }; For MIPS I've done some experiment with this and the result (with N64 ABI) is: With GCC trunk, G++ trunk, and GCC 11.2: argument passed via FPR $f12 and GPR $5, returned via GPR $2 and $3 With G++ 11.2: argument passed via FPR $f12 and $f13, returned via FPR $f0 and $f2 So I guess we need -Wpsabi for both mips_function_arg and mips_fpr_return_fields.
(In reply to Xi Ruoyao from comment #27) > (In reply to Jakub Jelinek from comment #23) > > > struct A { double a; int : 0; double b; }; > > For MIPS I've done some experiment with this and the result (with N64 ABI) > is: > > With GCC trunk, G++ trunk, and GCC 11.2: argument passed via FPR $f12 and > GPR $5, > returned via GPR $2 and $3 Ah, indeed. The for (; field; field = DECL_CHAIN (field)) if (TREE_CODE (field) == FIELD_DECL && int_bit_position (field) >= bitpos) break; loop will simply stop on the first field with bitpos equal or greater than bitpos, so the zero sized bit-fields (and generally any other zero sized fields like struct S {} s; in GNU C) will be treated as being part of the next slot. So in struct B { int : 0; double a, b; }; it will go into GPR and FPR, while struct C { double a; int : 0; double b; }; into FPR and GPR, and struct D { double a, b; int : 0; } into FPR & FPR. > With G++ 11.2: argument passed via FPR $f12 and $f13, returned via FPR $f0 > and $f2 > > So I guess we need -Wpsabi for both mips_function_arg and > mips_fpr_return_fields. Is there somebody who can clarify the MIPS ABI intent? Also, what does LLVM do?
> Is there somebody who can clarify the MIPS ABI intent? > Also, what does LLVM do? I've CC'ed Yunqiang and Fangrui. And I'll build clang for MIPS to see...
(In reply to Jakub Jelinek from comment #28) > Also, what does LLVM do? clang-14 agree with gcc-12 on the return values, as we expected (the ABI documentation is clear enough). But clang-14 treats arguments differently: struct foo { int : 0; double a; int : 0; double b; int : 0; }; extern void func(struct foo); void pass_foo(void) { struct foo test; test.a = 114; test.b = 514; func(test); } It puts "a" into $f12 and "b" into $f13. So the behavior of clang-14 and clang++-14 handling arguments with zero-width bit-fields is same as g++-11, and different from g++-12, gcc-11, and gcc-12. I'm not sure if we should keep our current behavior, or change it to match LLVM.
Well, what do other compilers do? It's not such a good idea to break ABI compatibility with the 1990's compilers ;-)
One of the reasons we changed the ABI on various arches one way or another is that we had this ABI incompatibility between C and C++, that just can't be right. At that time we can and should decide if we go for compatibility with C or C++. The MIPS n64 wording I could find: "Regardless of the struct field structure, it is treated as a sequence of 64-bit chunks. If a chunk consists solely of a double float field (but not a double, which is part of a union), it is passed in a floating point register. Any other chunk is passed in an integer register." is unclear for both the zero width bitfields and empty structure/union C members, members that have zero size and are on the 64-bit chunk boundaries can belong to either previous or next chunk or can be ignored altogether. I think best would be to ignore them altogether, especially if other compilers do that too. Also consider: struct T {}; struct S { struct T t; double a; struct T u; double b; }; struct U { int a[0]; double b; int c[0]; double d; }; argument passing in C (etc.) - in C++ struct T will have size 1 and therefore will be passed differently between C (which pedantically disallows empty structures or zero length arrays) and C++.
(In reply to Segher Boessenkool from comment #31) > Well, what do other compilers do? It's not such a good idea to break ABI > compatibility with the 1990's compilers ;-) Does someone have access to a Greenhills compiler here? (In reply to Jakub Jelinek from comment #32) > I think best would be to ignore them altogether, especially if other > compilers do that too. I agree the behavior of clang (or previous G++) is more "rational". To make things more "interesting": > So in struct B { int : 0; double a, b; }; it will go into GPR and FPR GCC trunk puts "a" into FPR, not GPR! So the "leading" zero-width bit-fields are ignored (GCC does not think it's a part of any "64-bit chunk"), but other zero-width bit-fields are accounted... This is just puzzling to me. I'll make a patch to match the behavior of clang.
(In reply to Xi Ruoyao from comment #33) > > So in struct B { int : 0; double a, b; }; it will go into GPR and FPR > > GCC trunk puts "a" into FPR, not GPR! So the "leading" zero-width > bit-fields are ignored (GCC does not think it's a part of any "64-bit > chunk"), but other zero-width bit-fields are accounted... This is just > puzzling to me. Remove this: I just forgot to rename ".C" to ".c" when I tested this with GCC 11. But still I think clang's behavior is better. > I'll make a patch to match the behavior of clang.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:e0ce885851dfd926c0cfe6f23a2debc87ea2bb9d commit r12-7951-ge0ce885851dfd926c0cfe6f23a2debc87ea2bb9d Author: Jakub Jelinek <jakub@redhat.com> Date: Fri Apr 1 11:49:40 2022 +0200 testsuite: Add further zero size elt passing tests [PR102024] As discussed in PR102024, zero width bitfields might not be the only ones causing ABI issues at least on mips, zero size arrays or (in C only) zero sized (empty) structures can be problematic too. The following patch adds some coverage for it too. Tested on x86_64-linux with make check-gcc check-g++ RUNTESTFLAGS='ALT_CC_UNDER_TEST=gcc ALT_CXX_UNDER_TEST=g++ --target_board=unix\{-m32,-m64\} compat.exp=pr102024*' make check-gcc check-g++ RUNTESTFLAGS='ALT_CC_UNDER_TEST=clang ALT_CXX_UNDER_TEST=clang++ --target_board=unix\{-m32,-m64\} compat.exp=pr102024*' with gcc/g++ 10.3 and clang 11. Everything but (expectedly) FAIL: gcc.dg/compat/pr102024 c_compat_x_tst.o-c_compat_y_alt.o execute FAIL: gcc.dg/compat/pr102024 c_compat_x_alt.o-c_compat_y_tst.o execute for -m64 ALT_CC_UNDER_TEST=gcc passes. 2022-04-01 Jakub Jelinek <jakub@redhat.com> PR target/102024 * gcc.dg/compat/pr102024_test.h: Add further tests with zero sized structures and arrays. * g++.dg/compat/pr102024_test.h: Add further tests with zero sized arrays.
The master branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>: https://gcc.gnu.org/g:0d4b97f1ee5213dffce107bc9f260a22fb23b4b1 commit r12-7961-g0d4b97f1ee5213dffce107bc9f260a22fb23b4b1 Author: Xi Ruoyao <xry111@mengyan1223.wang> Date: Wed Mar 30 22:22:45 2022 +0800 mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024] gcc/ PR target/102024 * config/mips/mips.cc (mips_fpr_return_fields): Detect C++ zero-width bit-fields and set up an indicator. (mips_return_in_msb): Adapt for mips_fpr_return_fields change. (mips_function_value_1): Diagnose when the presense of a C++ zero-width bit-field changes function returning in GCC 12. gcc/testsuite/ PR target/102024 * g++.target/mips/mips.exp: New test supporting file. * g++.target/mips/pr102024.C: New test.
The master branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>: https://gcc.gnu.org/g:413187b0b3c873333253838e4afbf8463b288b59 commit r12-7962-g413187b0b3c873333253838e4afbf8463b288b59 Author: Xi Ruoyao <xry111@mengyan1223.wang> Date: Thu Mar 31 23:40:23 2022 +0800 mips: Ignore zero width fields in arguments and issue -Wpsabi warning about C zero-width field ABI changes [PR102024] gcc/ PR target/102024 * config/mips/mips.cc (mips_function_arg): Ignore zero-width fields, and inform if it causes a psABI change. gcc/testsuite/ PR target/102024 * gcc.target/mips/pr102024-1.c: New test. * gcc.target/mips/pr102024-2.c: New test. * gcc.target/mips/pr102024-3.c: New test.
+ cat test.c struct foo { int : 0; double a; int : 0; double b; int : 0; }; extern void func(struct foo); void pass_foo(void) { struct foo test; test.a = 114; test.b = 514; func(test); } + cc -version MIPSpro Compilers: Version 7.30 + cc -64 -ansi -c test.c -o test.o Cannot find license file (-1,73:2) No such file or directory This product (cc) requires a license password. For license installation and trouble shooting information visit the web page: http://www.sgi.com/Support/Licensing/install_docs.html To obtain a Permanent license (proof of purchase required) or an Evaluation license please visit our license request web page: http://www.sgi.com/Products/license.html or send a blank email message to: license@sgi.com In North America, Silicon Graphics' customers may request Permanent licenses by sending a facsimile to: (650) 390-0537 or by calling our technical support hotline 1-800-800-4SGI If you are Outside of North America or you are not a Silicon Graphics support customer then contact your local support provider. qemu: Unsupported syscall: sgisys(107) qemu: Unsupported syscall: sgisys(111) qemu: Unsupported syscall: sgisys(107) qemu: Unsupported syscall: sgisys(111) ### Assertion failure at line 137 of ../../be/cg/MIPS/exp_loadstore.cxx: ### Compiler Error in file test.c during Code_Expansion phase: ### unexpected mtypes in Pick_Load_Instruction cc INTERNAL ERROR: /usr/lib32/cmplrs/be returned non-zero status 1
The master branch has been updated by Andreas Krebbel <krebbel@gcc.gnu.org>: https://gcc.gnu.org/g:bc79f0d9048375e402497d5f2ef457c9500310e4 commit r12-8277-gbc79f0d9048375e402497d5f2ef457c9500310e4 Author: Andreas Krebbel <krebbel@linux.ibm.com> Date: Wed Apr 27 09:20:41 2022 +0200 PR102024 - IBM Z: Add psabi diagnostics For IBM Z in particular there is a problem with structs like: struct A { float a; int :0; }; Our ABI document allows passing a struct in an FPR only if it has exactly one member. On the other hand it says that structs of 1,2,4,8 bytes are passed in a GPR. So this struct is expected to be passed in a GPR. Since we don't return structs in registers (regardless of the number of members) it is always returned in memory. Situation is as follows: All compiler versions tested return it in memory - as expected. gcc 11, gcc 12, g++ 12, and clang 13 pass it in a GPR - as expected. g++ 11 as well as clang++ 13 pass in an FPR For IBM Z we stick to the current GCC 12 behavior, i.e. zero-width bitfields are NOT ignored. A struct as above will be passed in a GPR. Rational behind this is that not affecting the C ABI is more important here. A patch for clang is in progress: https://reviews.llvm.org/D122388 In addition to the usual regression test I ran the compat and struct-layout-1 testsuites comparing the compiler before and after the patch. gcc/ChangeLog: PR target/102024 * config/s390/s390-protos.h (s390_function_arg_vector): Remove prototype. * config/s390/s390.cc (s390_single_field_struct_p): New function. (s390_function_arg_vector): Invoke s390_single_field_struct_p. (s390_function_arg_float): Likewise. gcc/testsuite/ChangeLog: PR target/102024 * g++.target/s390/pr102024-1.C: New test. * g++.target/s390/pr102024-2.C: New test. * g++.target/s390/pr102024-3.C: New test. * g++.target/s390/pr102024-4.C: New test. * g++.target/s390/pr102024-5.C: New test. * g++.target/s390/pr102024-6.C: New test.