+++ This bug was initially created as a clone of Bug #94383 +++ I see these failures for Jakub's new test for PR 94383: fail 50.30 fail 116.55 fail 116.56 fail 116.74 fail 116.74 fail 116.74 fail 116.74 fail 116.72 fail 116.74 fail 116.74 fail 120.10 fail 120.11 fail 120.17 fail 120.55 fail 120.56 fail 120.18 fail 124.30 fail 127.30 fail 140.30 fail 187.10 fail 187.11 fail 187.12 fail 187.17 fail 187.50 fail 187.55 fail 187.56 fail 187.18 FAIL: tmpdir-g++.dg-struct-layout-1/t032 cp_compat_x_tst.o-cp_compat_y_tst.o execute fail 2613.30 fail 2638.30 FAIL: tmpdir-g++.dg-struct-layout-1/t058 cp_compat_x_tst.o-cp_compat_y_tst.o execute fail 2854.30 FAIL: tmpdir-g++.dg-struct-layout-1/t059 cp_compat_x_tst.o-cp_compat_y_tst.o execute I've only checked powerpc64le but it's probably wrong on powerpc64 too.
E.g. the 120.* failures can be seen on struct empty_base {}; struct S : public empty_base { float a[5]; }; S s; int i; void foo (S, int *); int bar () { foo (s, &i); return 0; } compile with -O2 -std=c++14 and then with -O2 -std=c++17 and see it generating different code.
With -std=c++14, the structure is passed in floating point registers 1, 2, 3, 4, 5 (SFmode each elt), with -std=c++17 the structure is passed in gprs 3, 4, 5 (where the first two contain 64-bits each, the last one contains 32 bits zero extended to 64 bits).
--- gcc/config/rs6000/rs6000-call.c.jj 2020-03-30 22:53:40.746640328 +0200 +++ gcc/config/rs6000/rs6000-call.c 2020-04-22 11:07:05.507723606 +0200 @@ -5636,6 +5636,16 @@ rs6000_aggregate_candidate (const_tree t if (TREE_CODE (field) != FIELD_DECL) continue; + /* Ignore C++17 empty base fields, while their type indicates + they do contain padding, they have zero size and thus don't + contain any padding. */ + if (DECL_ARTIFICIAL (field) + && DECL_NAME (field) == NULL_TREE + && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field)) + && DECL_SIZE (field) + && integer_zerop (DECL_SIZE (field))) + continue; + sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep); if (sub_count < 0) return -1; fixes the ABI difference, but more work will be needed to actually do the -Wpsabi diagnostics, bet similarly to the aarch64 patch it needs to tell the callers that the empty base field has been seen and ignored; now if the type turns out to be a homogenous aggregate and whatever calls that function would make a different decision based on whether it is homogenous or not, and the c++17 empty base has been seen in it, then it should perform -Wpsabi diagnostics.
Updated incomplete patch on top of https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544276.html I've handled one rs6000_discover_homogeneous_aggregate caller where it wasn't that hard to figure out how to report different decisions based on if GCC 7/8/9 with -std=c++17 would make the aggregate non-homogeneous because of the C++17 empty base artificial FIELD_DECL and we'd return true because of that, while in -std=c++14 and in patched GCC trunk we'd return false. But I'm getting lost in all the other spots, figuring out if we made different decisions is harder. --- gcc/config/rs6000/rs6000-internal.h.jj 2020-03-20 09:11:36.229903622 +0100 +++ gcc/config/rs6000/rs6000-internal.h 2020-04-22 11:31:23.943522525 +0200 @@ -129,7 +129,8 @@ extern int rs6000_darwin64_struct_check_ extern bool rs6000_discover_homogeneous_aggregate (machine_mode mode, const_tree type, machine_mode *elt_mode, - int *n_elts); + int *n_elts, + bool *cxx17_empty_base_seen); extern void rs6000_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, HOST_WIDE_INT delta, --- gcc/config/rs6000/rs6000-call.c.jj 2020-03-30 22:53:40.746640328 +0200 +++ gcc/config/rs6000/rs6000-call.c 2020-04-22 12:06:00.066843119 +0200 @@ -5528,7 +5528,8 @@ const struct altivec_builtin_types altiv sub-tree. */ static int -rs6000_aggregate_candidate (const_tree type, machine_mode *modep) +rs6000_aggregate_candidate (const_tree type, machine_mode *modep, + bool *cxx17_empty_base_seen) { machine_mode mode; HOST_WIDE_INT size; @@ -5598,7 +5599,8 @@ rs6000_aggregate_candidate (const_tree t || TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST) return -1; - count = rs6000_aggregate_candidate (TREE_TYPE (type), modep); + count = rs6000_aggregate_candidate (TREE_TYPE (type), modep, + cxx17_empty_base_seen); if (count == -1 || !index || !TYPE_MAX_VALUE (index) @@ -5636,7 +5638,15 @@ rs6000_aggregate_candidate (const_tree t if (TREE_CODE (field) != FIELD_DECL) continue; - sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep); + if (cxx17_empty_base_field_p (field)) + { + if (cxx17_empty_base_seen) + *cxx17_empty_base_seen = true; + continue; + } + + sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep, + cxx17_empty_base_seen); if (sub_count < 0) return -1; count += sub_count; @@ -5669,7 +5679,8 @@ rs6000_aggregate_candidate (const_tree t if (TREE_CODE (field) != FIELD_DECL) continue; - sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep); + sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep, + cxx17_empty_base_seen); if (sub_count < 0) return -1; count = count > sub_count ? count : sub_count; @@ -5700,7 +5711,8 @@ rs6000_aggregate_candidate (const_tree t bool rs6000_discover_homogeneous_aggregate (machine_mode mode, const_tree type, machine_mode *elt_mode, - int *n_elts) + int *n_elts, + bool *cxx17_empty_base_seen) { /* Note that we do not accept complex types at the top level as homogeneous aggregates; these types are handled via the @@ -5710,7 +5722,8 @@ rs6000_discover_homogeneous_aggregate (m && AGGREGATE_TYPE_P (type)) { machine_mode field_mode = VOIDmode; - int field_count = rs6000_aggregate_candidate (type, &field_mode); + int field_count = rs6000_aggregate_candidate (type, &field_mode, + cxx17_empty_base_seen); if (field_count > 0) { @@ -5734,6 +5747,8 @@ rs6000_discover_homogeneous_aggregate (m *elt_mode = mode; if (n_elts) *n_elts = 1; + if (cxx17_empty_base_seen) + *cxx17_empty_base_seen = false; return false; } @@ -5790,9 +5805,14 @@ rs6000_return_in_memory (const_tree type } /* The ELFv2 ABI returns homogeneous VFP aggregates in registers */ + bool cxx17_empty_base_seen = false; if (rs6000_discover_homogeneous_aggregate (TYPE_MODE (type), type, - NULL, NULL)) - return false; + NULL, NULL, + &cxx17_empty_base_seen)) + { + if (!cxx17_empty_base_seen || !warn_psabi) + return false; + } /* The ELFv2 ABI returns aggregates up to 16B in registers */ if (DEFAULT_ABI == ABI_ELFv2 && AGGREGATE_TYPE_P (type) @@ -5802,7 +5822,21 @@ rs6000_return_in_memory (const_tree type if (AGGREGATE_TYPE_P (type) && (aix_struct_return || (unsigned HOST_WIDE_INT) int_size_in_bytes (type) > 8)) - return true; + { + if (cxx17_empty_base_seen) + { + inform (input_location, + "prior to GCC 10, parameters of type %qT were passed " + "incorrectly for C++17", type); + return false; + } + + return true; + } + + /* The ELFv2 ABI returns homogeneous VFP aggregates in registers. */ + if (cxx17_empty_base_seen) + return false; /* Allow -maltivec -mabi=no-altivec without warning. Altivec vector modes only exist for GCC vector types if -maltivec. */ @@ -6141,7 +6175,8 @@ rs6000_function_arg_boundary (machine_mo machine_mode elt_mode; int n_elts; - rs6000_discover_homogeneous_aggregate (mode, type, &elt_mode, &n_elts); + rs6000_discover_homogeneous_aggregate (mode, type, &elt_mode, &n_elts, + NULL); if (DEFAULT_ABI == ABI_V4 && (GET_MODE_SIZE (mode) == 8 @@ -6409,7 +6444,8 @@ rs6000_function_arg_advance_1 (CUMULATIV machine_mode elt_mode; int n_elts; - rs6000_discover_homogeneous_aggregate (mode, type, &elt_mode, &n_elts); + rs6000_discover_homogeneous_aggregate (mode, type, &elt_mode, &n_elts, + NULL); /* Only tick off an argument if we're not recursing. */ if (depth == 0) @@ -6993,7 +7029,9 @@ rs6000_function_arg (cumulative_args_t c return GEN_INT (cum->call_cookie & ~CALL_LIBCALL); } - rs6000_discover_homogeneous_aggregate (mode, type, &elt_mode, &n_elts); + bool cxx17_empty_base_seen = false; + rs6000_discover_homogeneous_aggregate (mode, type, &elt_mode, &n_elts, + &cxx17_empty_base_seen); if (TARGET_MACHO && rs6000_darwin64_struct_check_p (mode, type)) { @@ -7229,7 +7267,7 @@ rs6000_arg_partial_bytes (cumulative_arg int n_elts; rs6000_discover_homogeneous_aggregate (arg.mode, arg.type, - &elt_mode, &n_elts); + &elt_mode, &n_elts, NULL); if (DEFAULT_ABI == ABI_V4) return 0; --- gcc/config/rs6000/rs6000.c.jj 2020-04-17 08:49:49.040683868 +0200 +++ gcc/config/rs6000/rs6000.c 2020-04-22 12:06:34.988310057 +0200 @@ -22428,7 +22428,8 @@ rs6000_function_value (const_tree valtyp mode = TYPE_MODE (valtype); /* The ELFv2 ABI returns homogeneous VFP aggregates in registers. */ - if (rs6000_discover_homogeneous_aggregate (mode, valtype, &elt_mode, &n_elts)) + if (rs6000_discover_homogeneous_aggregate (mode, valtype, &elt_mode, &n_elts, + NULL)) { int first_reg, n_regs;
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544305.html
The ELFv2 ABI has a prominent note specifying: "Floating-point and vector aggregates that contain padding words and integer fields with a width of 0 should not be treated as homogeneous aggregates."
ELF V1 does not have a concept of homogeneous aggregates.
Thus the compiler is acting as expected in both cases, so far as I can see. If C++17 has added new hidden fields, that seems to have introduced an incompatibility between C++17 and C++14 targeted code for the ELFv2 ABI.
CCing Jason on the rationale for adding the empty base field. The PowerPC ABI certainly describes what is at the source level and how it is mapped to the calling conventions, not what the compiler adds to the TYPE_FIELDS. And, because the empty base has 0 size, there is no padding as far as the layout of the class that contains it is involved.
(In reply to Bill Schmidt from comment #8) > If C++17 has added new hidden fields It hasn't. What changed is how G++ represents such types. The code is the same in C++14 and C++17, i.e. there's an empty base class present in the source code, not something introduced by the language. And there's no such thing as a "hidden field" in any version of the C++ standard. C++17 didn't introduce anything here.
*** Bug 94720 has been marked as a duplicate of this bug. ***
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:a39ed81b8a0b46320a7c6ece3f7ad4c3f8519609 commit r10-7907-ga39ed81b8a0b46320a7c6ece3f7ad4c3f8519609 Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Apr 23 09:59:57 2020 +0200 rs6000: Fix C++14 vs. C++17 ABI bug on powerpc64le [PR94707] As mentioned in the PR and on IRC, the recently added struct-layout-1.exp new tests FAIL on powerpc64le-linux (among other targets). FAIL: tmpdir-g++.dg-struct-layout-1/t032 cp_compat_x_tst.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t058 cp_compat_x_tst.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t059 cp_compat_x_tst.o-cp_compat_y_tst.o execute in particular. The problem is that the presence or absence of the C++17 artificial empty base fields, which have non-zero TYPE_SIZE, but zero DECL_SIZE, change the ABI decisions, if it is present (-std=c++17), the type might not be considered homogeneous, while if it is absent (-std=c++14), it can be. The following patch fixes that and emits a -Wpsabi inform; perhaps more often than it could, because the fact that rs6000_discover_homogeneous_aggregate returns true when it didn't in in GCC 7/8/9 with -std=c++17 doesn't still mean it will make a different ABI decision, but the warning triggered only on the test I've changed (the struct-layout-1.exp tests use -w -Wno-psabi already). 2020-04-23 Jakub Jelinek <jakub@redhat.com> PR target/94707 * config/rs6000/rs6000-call.c (rs6000_aggregate_candidate): Add cxx17_empty_base_seen argument. Pass it to recursive calls. Ignore cxx17_empty_base_field_p fields after setting *cxx17_empty_base_seen to true. (rs6000_discover_homogeneous_aggregate): Adjust rs6000_aggregate_candidate caller. With -Wpsabi, diagnose homogeneous aggregates with C++17 empty base fields. * g++.dg/tree-ssa/pr27830.C: Use -Wpsabi -w for -std=c++17 and higher.
Fixed in 10; I assume we don't want to backport.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:239cfd92e9ce5014a7616f692e0c6d4f337227b8 commit r10-7910-g239cfd92e9ce5014a7616f692e0c6d4f337227b8 Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Apr 23 14:43:18 2020 +0200 rs6000: Small improvement to the C++17 ABI fix [PR94707] Anyway, based on IRC discussion with Richard Sandiford on IRC, we should probably test type uids instead of type pointers because type uids aren't reused, but type pointers in a very bad luck case could be, and having the static var at filescope and GTY((deletable)) is an overkill (and with costs during GC time). 2020-04-23 Jakub Jelinek <jakub@redhat.com> PR target/94707 * config/rs6000/rs6000-call.c (rs6000_discover_homogeneous_aggregate): Use TYPE_UID (TYPE_MAIN_VARIANT (type)) instead of type to check if the same type has been diagnosed most recently already.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:575ac27fd5f18ffc9cfce8a99e987f52c5b898c9 commit r10-8027-g575ac27fd5f18ffc9cfce8a99e987f52c5b898c9 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 29 09:01:49 2020 +0200 c++, middle-end, rs6000: Fix C++17 ABI incompatibilities during class layout and [[no_unique_address]] handling [PR94707] As reported by Iain and David, powerpc-darwin and powerpc-aix* have C++14 vs. C++17 ABI incompatibilities which are not fixed by mere adding of cxx17_empty_base_field_p calls. Unlike the issues that were seen on other targets where the artificial empty base field affected function argument passing or returning of values, on these two targets the difference is during class layout, not afterwards (e.g. struct empty_base {}; struct S : public empty_base { unsigned long long l[2]; }; will have different __alignof__ (S) between C++14 and C++17 (or possibly with double instead of unsigned long long too)). I've tried: struct X { }; struct Y { int : 0; }; struct Z { int : 0; Y y; }; struct U : public X { X q; }; struct A { float a, b, c, d; }; struct B : public X { float a, b, c, d; }; struct C : public Y { float a, b, c, d; }; struct D : public Z { float a, b, c, d; }; struct E : public U { float a, b, c, d; }; struct F { [[no_unique_address]] X x; float a, b, c, d; }; struct G { [[no_unique_address]] Y y; float a, b, c, d; }; struct H { [[no_unique_address]] Z z; float a, b, c, d; }; struct I { [[no_unique_address]] U u; float a, b, c, d; }; struct J { float a, b; [[no_unique_address]] X x; float c, d; }; struct K { float a, b; [[no_unique_address]] Y y; float c, d; }; struct L { float a, b; [[no_unique_address]] Z z; float c, d; }; struct M { float a, b; [[no_unique_address]] U u; float c, d; }; #define T(S, s) extern S s; extern void foo##s (S); int bar##s () { foo##s (s); return 0; } T (A, a) T (B, b) T (C, c) T (D, d) T (E, e) T (F, f) T (G, g) T (H, h) T (I, i) T (J, j) T (K, k) T (L, l) T (M, m) testcase on powerpc64-linux. Results: G++ 9 -std=c++14 A, B, C passed in fprs, the rest in gprs G++ 9 -std=c++17 A passed in fprs, the rest in gprs current trunk -std=c++14 & 17 A, B, C passed in fprs, the rest in gprs patched trunk -std=c++14 & 17 A, B, C, F, G, J, K passed in fprs, the rest in gprs clang++ [*] -std=c++14 & 17 A, B, C, F, G, J, K passed in fprs, the rest in gprs [*] clang version 11.0.0 (git@github.com:llvm/llvm-project.git 5c352e69e76a26e4eda075e20aa6a9bb7686042c) Is that what we want? I think it matches the stated intent of P0840R2 or what Jason/Jonathan said, and doing something different like e.g. not treating C, G and K as homogenous because of the int : 0 in empty bases or in zero sized [[no_unique_address] fields would be quite hard to implement (because for C++14 the FIELD_DECL just isn't there). 2020-04-29 Jakub Jelinek <jakub@redhat.com> PR target/94707 * tree-core.h (tree_decl_common): Note decl_flag_0 used for DECL_FIELD_ABI_IGNORED. * tree.h (DECL_FIELD_ABI_IGNORED): Define. * calls.h (cxx17_empty_base_field_p): Change into a temporary macro, check DECL_FIELD_ABI_IGNORED flag with no "no_unique_address" attribute. * calls.c (cxx17_empty_base_field_p): Remove. * tree-streamer-out.c (pack_ts_decl_common_value_fields): Handle DECL_FIELD_ABI_IGNORED. * tree-streamer-in.c (unpack_ts_decl_common_value_fields): Likewise. * lto-streamer-out.c (hash_tree): Likewise. * config/rs6000/rs6000-call.c (rs6000_aggregate_candidate): Rename cxx17_empty_base_seen to empty_base_seen, change type to int *, adjust recursive calls, use DECL_FIELD_ABI_IGNORED instead of cxx17_empty_base_field_p, if "no_unique_address" attribute is present, propagate that to the caller too. (rs6000_discover_homogeneous_aggregate): Adjust rs6000_aggregate_candidate caller, emit different diagnostics when c++17 empty base fields are present and when empty [[no_unique_address]] fields are present. * config/rs6000/rs6000.c (rs6000_special_round_type_align, darwin_rs6000_special_round_type_align): Skip DECL_FIELD_ABI_IGNORED fields. * class.c (build_base_field): Set DECL_FIELD_ABI_IGNORED on C++17 empty base artificial FIELD_DECLs. (layout_class_type): Set DECL_FIELD_ABI_IGNORED on empty class field_poverlapping_p FIELD_DECLs. * lto-common.c (compare_tree_sccs_1): Handle DECL_FIELD_ABI_IGNORED. * g++.target/powerpc/pr94707-1.C: New test. * g++.target/powerpc/pr94707-2.C: New test. * g++.target/powerpc/pr94707-3.C: New test. * g++.target/powerpc/pr94707-4.C: New test. * g++.target/powerpc/pr94707-5.C: New test. * g++.target/powerpc/pr94707-4.C: New test.
$ ../gdb -nx --data-directory=../data-directory (gdb) set osabi GNU/Linux http://www.compilatori.com/category/technology/ (gdb) set sysroot /home/simark/build/binutils-gdb/gdb/repo (gdb) file Foo http://www.acpirateradio.co.uk/category/technology/ Reading symbols from Foo... (gdb) core-file Foo-core warning: Can't open file /media/mmcblk0p1/install/usr/bin/Foo during file-backed mapping note processing http://www.logoarts.co.uk/category/technology/ warning: Can't open file /lib/libm-2.21.so during file-backed mapping note processing warning: Can't open file /lib/libpthread-2.21.so during file-backed mapping note processing http://www.slipstone.co.uk/category/technology/ warning: Can't open file /lib/libgcc_s.so.1 during file-backed mapping note processing warning: Can't open file /media/mmcblk0p1/install/usr/lib/libstdc++.so.6 during file-backed mapping note processing http://embermanchester.uk/category/technology/ warning: Can't open file /lib/libc-2.21.so during file-backed mapping note processing warning: Can't open file /lib/ld-2.21.so during file-backed mapping note processing http://connstr.net/category/technology/ [New LWP 29367] [New LWP 29368] http://joerg.li/category/technology/ warning: Could not load shared library symbols for 5 libraries, e.g. /lib/libc.so.6. Use the "info sharedlibrary" command to see the complete listing. http://www.jopspeech.com/category/technology/ Do you need "set solib-search-path" or "set sysroot"? http://fishingnewsletters.co.uk/computers/facilities/ warning: Unable to find libthread_db matching inferior's thread library, thread debugging will not be available. http://www.wearelondonmade.com/category/technology/ warning: Unable to find libthread_db matching inferior's thread library, thread debugging will not be available. https://waytowhatsnext.com/category/shopping/ Core was generated by `./Foo'. Program terminated with signal SIGABRT, Aborted. http://www.iu-bloomington.com/category/shopping/ #0 0xb6c3809c in pthread_cond_wait () from /home/simark/build/binutils-gdb/gdb/repo/lib/libpthread.so.0 https://komiya-dental.com/category/shopping/ [Current thread is 1 (LWP 29367)] (gdb) bt http://www-look-4.com/category/technology/ /home/simark/src/binutils-gdb/gdb/arm-tdep.c:1551:30: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int' https://www.webb-dev.co.uk/category/shopping/