| Summary: | [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on s390x/s390 | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Jakub Jelinek <jakub> |
| Component: | target | Assignee: | Andreas Krebbel <krebbel> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | CC: | jakub, krebbel, ktkachov, marxin, matmal01, mpolacek, rearnsha, redi, rguenth, rsandifo |
| Priority: | P1 | Keywords: | ABI, wrong-code |
| Version: | 10.0 | ||
| Target Milestone: | 10.0 | ||
| Host: | Target: | s390*-*-linux | |
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2020-04-22 00:00:00 | |
| Bug Depends on: | 94383 | ||
| Bug Blocks: | |||
|
Description
Jakub Jelinek
2020-04-22 07:05:54 UTC
Completely untested guess:
--- gcc/config/s390/s390.c.jj 2020-03-14 08:14:47.097741411 +0100
+++ gcc/config/s390/s390.c 2020-04-22 14:24:17.980091105 +0200
@@ -11917,7 +11917,8 @@ s390_function_arg_vector (machine_mode m
for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
{
- if (TREE_CODE (field) != FIELD_DECL)
+ if (TREE_CODE (field) != FIELD_DECL
+ || cxx17_empty_base_field_p (field))
continue;
if (single == NULL_TREE)
@@ -11967,7 +11968,8 @@ s390_function_arg_float (machine_mode mo
for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
{
- if (TREE_CODE (field) != FIELD_DECL)
+ if (TREE_CODE (field) != FIELD_DECL
+ || cxx17_empty_base_field_p (field))
continue;
if (single == NULL_TREE)
(on top of the today posted cxx17_empty_base_field_p patch).
Though, of course that doesn't handle the -Wpsabi part if s390{,x} wants to emit such diagnostics; for that the functions or their caller should determine
if they would make a different ABI decisions if the field wouldn't be skipped.
(In reply to Jakub Jelinek from comment #1) You patch fixes the testcases on IBM Z. I'll run a full bootstrap test over night. The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:9612a4833d761e3beda083a3e4dc92feba3b01bc commit r10-7985-g9612a4833d761e3beda083a3e4dc92feba3b01bc Author: Jakub Jelinek <jakub@redhat.com> Date: Mon Apr 27 09:11:57 2020 +0200 s390: Fix C++14 vs. C++17 ABI incompatibility on s390{,x} [PR94704] The following patch fixes the C++14 vs. C++17 ABI passing incompatibility on s390x-linux. Bootstrapped/regtested on s390x-linux without and with the patch, the difference being: -FAIL: tmpdir-g++.dg-struct-layout-1/t032 cp_compat_x_alt.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t032 cp_compat_x_tst.o-cp_compat_y_alt.o execute -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/t055 cp_compat_x_alt.o-cp_compat_y_alt.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t055 cp_compat_x_alt.o-cp_compat_y_tst.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t055 cp_compat_x_tst.o-cp_compat_y_alt.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t055 cp_compat_x_tst.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t056 cp_compat_x_alt.o-cp_compat_y_alt.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t056 cp_compat_x_alt.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t056 cp_compat_x_tst.o-cp_compat_y_alt.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t056 cp_compat_x_tst.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t057 cp_compat_x_alt.o-cp_compat_y_alt.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t057 cp_compat_x_alt.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t057 cp_compat_x_tst.o-cp_compat_y_alt.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t057 cp_compat_x_tst.o-cp_compat_y_tst.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t058 cp_compat_x_alt.o-cp_compat_y_alt.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t058 cp_compat_x_alt.o-cp_compat_y_tst.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t058 cp_compat_x_tst.o-cp_compat_y_alt.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_alt.o-cp_compat_y_alt.o execute FAIL: tmpdir-g++.dg-struct-layout-1/t059 cp_compat_x_alt.o-cp_compat_y_tst.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t059 cp_compat_x_tst.o-cp_compat_y_alt.o execute -FAIL: tmpdir-g++.dg-struct-layout-1/t059 cp_compat_x_tst.o-cp_compat_y_tst.o execute when performing ALT_CXX_UNDER_TEST=g++ testing with a system GCC 10 compiler from a week ago. So, the alt vs. alt FAILs are all expected (we know before this patch there is an ABI incompatibility) and some alt vs. tst (or tst vs. alt) FAILs too - that depends on if the particular x or y test is compiled with -std=c++14 or -std=c++17 - if x_tst is compiled with -std=c++14 and y_alt is compiled with -std=c++17, then it should FAIL, similarly if x_alt is compiled with -std=c++17 and y_tst is compiled with -std=c++14. 2020-04-27 Jakub Jelinek <jakub@redhat.com> PR target/94704 * config/s390/s390.c (s390_function_arg_vector, s390_function_arg_float): Ignore cxx17_empty_base_field_p fields. Fixed on the trunk, should not be backported as it is an ABI change. The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:dde5ce541e3258276848aee85229a71c0e5f6965 commit r10-8007-gdde5ce541e3258276848aee85229a71c0e5f6965 Author: Jakub Jelinek <jakub@redhat.com> Date: Tue Apr 28 10:26:24 2020 +0200 s390: -Wpsabi diagnostics for C++14 vs. C++17 ABI incompatibility on s390{,x} [PR94704] > We probably have to look into providing a -Wpsabi warning as well. So like this? 2020-04-28 Jakub Jelinek <jakub@redhat.com> PR target/94704 * config/s390/s390.c (s390_function_arg_vector, s390_function_arg_float): Emit -Wpsabi diagnostics if the ABI changed. The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:48e54fea7ba4a7cb7b3d1505951383120220e394 commit r10-8057-g48e54fea7ba4a7cb7b3d1505951383120220e394 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Apr 29 22:38:01 2020 +0200 s390: Fix up -Wpsabi diagnostics + [[no_unique_address]] empty member fix [PR94704] So, based on the yesterday's discussions, similarly to powerpc64le-linux I've done some testing for s390x-linux too. First of all, I found a bug in my patch from yesterday, it was printing the wrong type like 'double' etc. rather than the class that contained such the element. Fix below. For s390x-linux, I was using struct X { }; struct Y { int : 0; }; struct Z { int : 0; Y y; }; struct U : public X { X q; }; struct A { double a; }; struct B : public X { double a; }; struct C : public Y { double a; }; struct D : public Z { double a; }; struct E : public U { double a; }; struct F { [[no_unique_address]] X x; double a; }; struct G { [[no_unique_address]] Y y; double a; }; struct H { [[no_unique_address]] Z z; double a; }; struct I { [[no_unique_address]] U u; double a; }; struct J { double a; [[no_unique_address]] X x; }; struct K { double a; [[no_unique_address]] Y y; }; struct L { double a; [[no_unique_address]] Z z; }; struct M { double a; [[no_unique_address]] U u; }; #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) as testcase and looking for "\tld\t%f0,". While g++ 9 with -std=c++17 used to pass in fpr just A, g++ 9 -std=c++14, as well as current trunk -std=c++14 & 17 and clang++ from today -std=c++14 & 17 all pass A, B, C in fpr and nothing else. The intent stated by Jason seems to be that A, B, C, F, G, J, K should all be passed in fpr. Attached are two (updated) versions of the patch on top of the powerpc+middle-end patch just posted. The first one emits two separate -Wpsabi warnings like powerpc, one for the -std=c++14 vs. -std=c++17 ABI difference and one for GCC 9 vs. 10 [[no_unique_address]] passing changes, the other one is silent about the second case. 2020-04-29 Jakub Jelinek <jakub@redhat.com> PR target/94704 * config/s390/s390.c (s390_function_arg_vector, s390_function_arg_float): Use DECL_FIELD_ABI_IGNORED instead of cxx17_empty_base_field_p. In -Wpsabi diagnostics use the type passed to the function rather than the type of the single element. Rename cxx17_empty_base_seen variable to empty_base_seen, change type to int, and adjust diagnostics depending on if the field has [[no_unique_attribute]] or not. * g++.target/s390/s390.exp: New file. * g++.target/s390/pr94704-1.C: New test. * g++.target/s390/pr94704-2.C: New test. * g++.target/s390/pr94704-3.C: New test. * g++.target/s390/pr94704-4.C: New test. |