* [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64
@ 2022-04-06 12:33 Xi Ruoyao
2022-04-06 12:44 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2022-04-06 12:33 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek, Richard Sandiford, YunQiang Su
Another MIPS function return ABI fix. Ok for trunk?
--
This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure. Clang++
ignores C++17 empty bases in return values as well.
gcc/
* config/mips/mips.cc (mips_fpr_return_fields): Ignore
cxx17_empty_base_field_p fields.
---
gcc/config/mips/mips.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 0f2492219f3..5010f99f761 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6337,7 +6337,8 @@ mips_fpr_return_fields (const_tree valtype, tree *fields,
i = 0;
for (field = TYPE_FIELDS (valtype); field != 0; field = DECL_CHAIN (field))
{
- if (TREE_CODE (field) != FIELD_DECL)
+ if (TREE_CODE (field) != FIELD_DECL
+ || cxx17_empty_base_field_p (field))
continue;
if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64
2022-04-06 12:33 [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64 Xi Ruoyao
@ 2022-04-06 12:44 ` Jakub Jelinek
2022-04-06 12:48 ` Xi Ruoyao
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2022-04-06 12:44 UTC (permalink / raw)
To: Xi Ruoyao; +Cc: gcc-patches, Richard Sandiford, Jakub Jelinek, YunQiang Su
On Wed, Apr 06, 2022 at 08:33:40PM +0800, Xi Ruoyao via Gcc-patches wrote:
> Another MIPS function return ABI fix. Ok for trunk?
>
> --
>
> This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure. Clang++
> ignores C++17 empty bases in return values as well.
>
> gcc/
> * config/mips/mips.cc (mips_fpr_return_fields): Ignore
> cxx17_empty_base_field_p fields.
> ---
> gcc/config/mips/mips.cc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> index 0f2492219f3..5010f99f761 100644
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -6337,7 +6337,8 @@ mips_fpr_return_fields (const_tree valtype, tree *fields,
> i = 0;
> for (field = TYPE_FIELDS (valtype); field != 0; field = DECL_CHAIN (field))
> {
> - if (TREE_CODE (field) != FIELD_DECL)
> + if (TREE_CODE (field) != FIELD_DECL
> + || cxx17_empty_base_field_p (field))
> continue;
>
> if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
Well, this won't diagnose the ABI change.
So, if cxx17_empty_base_field_p, it should set some flag before continuing
and if it is considered a fpr return and that flag is set, it should emit a
-Wpsabi warning too.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64
2022-04-06 12:44 ` Jakub Jelinek
@ 2022-04-06 12:48 ` Xi Ruoyao
2022-04-06 13:44 ` [PATCH v2] " Xi Ruoyao
0 siblings, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2022-04-06 12:48 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Richard Sandiford, Jakub Jelinek, YunQiang Su
On Wed, 2022-04-06 at 14:44 +0200, Jakub Jelinek wrote:
> On Wed, Apr 06, 2022 at 08:33:40PM +0800, Xi Ruoyao via Gcc-patches wrote:
> > Another MIPS function return ABI fix. Ok for trunk?
> >
> > --
> >
> > This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure. Clang++
> > ignores C++17 empty bases in return values as well.
> >
> > gcc/
> > * config/mips/mips.cc (mips_fpr_return_fields): Ignore
> > cxx17_empty_base_field_p fields.
> > ---
> > gcc/config/mips/mips.cc | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> > index 0f2492219f3..5010f99f761 100644
> > --- a/gcc/config/mips/mips.cc
> > +++ b/gcc/config/mips/mips.cc
> > @@ -6337,7 +6337,8 @@ mips_fpr_return_fields (const_tree valtype, tree *fields,
> > i = 0;
> > for (field = TYPE_FIELDS (valtype); field != 0; field = DECL_CHAIN (field))
> > {
> > - if (TREE_CODE (field) != FIELD_DECL)
> > + if (TREE_CODE (field) != FIELD_DECL
> > + || cxx17_empty_base_field_p (field))
> > continue;
> >
> > if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
>
> Well, this won't diagnose the ABI change.
> So, if cxx17_empty_base_field_p, it should set some flag before continuing
> and if it is considered a fpr return and that flag is set, it should emit a
> -Wpsabi warning too.
Ok, will add it.
When I learnt from PR94704 fix I failed to notice the second commit
adding -Wpsabi warning :(.
--
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64
2022-04-06 12:48 ` Xi Ruoyao
@ 2022-04-06 13:44 ` Xi Ruoyao
2022-04-06 14:34 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2022-04-06 13:44 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Sandiford, gcc-patches, Jakub Jelinek, YunQiang Su
v2: Add psABI warning and test.
--
This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure. Clang++
also ignores C++17 empty bases in return values.
gcc/
* config/mips/mips.cc (mips_fpr_return_fields): Ignore
cxx17_empty_base_field_p fields and set an indicator.
(mips_return_in_msb): Adjust for mips_fpr_return_fields change.
(mips_function_value_1): Inform psABI change about C++ 17 empty
bases.
gcc/testsuite/
* g++.target/mips/cxx17_empty_base.C: New test.
---
gcc/config/mips/mips.cc | 58 +++++++++++++++++--
.../g++.target/mips/cxx17_empty_base.C | 20 +++++++
2 files changed, 73 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.target/mips/cxx17_empty_base.C
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 079bb03968a..57ce0884951 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6346,12 +6346,21 @@ mips_callee_copies (cumulative_args_t, const function_arg_info &arg)
The C++ FE used to remove zero-width bit-fields in GCC 11 and earlier.
To make a proper diagnostic, this function will set
HAS_CXX_ZERO_WIDTH_BF to true once a C++ zero-width bit-field shows up,
- and then ignore it. Then the caller can determine if this zero-width
- bit-field will make a difference and emit a -Wpsabi inform. */
+ and then ignore it.
+
+ We had failed to ignore C++ 17 empty bases in GCC 7, 8, 9, 10, and 11.
+ This caused an ABI incompatibility between C++ 14 and C++ 17. This is
+ fixed now and to make a proper diagnostic, this function will set
+ HAS_CXX17_EMPTY_BASE to true once a C++ 17 empty base shows up, and
+ then ignore it.
+
+ The caller should use the value of HAS_CXX17_EMPTY_BASE and/or
+ HAS_CXX_ZERO_WIDTH_BF to emit a proper -Wpsabi inform. */
static int
mips_fpr_return_fields (const_tree valtype, tree *fields,
- bool *has_cxx_zero_width_bf)
+ bool *has_cxx_zero_width_bf,
+ bool *has_cxx17_empty_base)
{
tree field;
int i;
@@ -6368,6 +6377,12 @@ mips_fpr_return_fields (const_tree valtype, tree *fields,
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ if (cxx17_empty_base_field_p (field))
+ {
+ *has_cxx17_empty_base = true;
+ continue;
+ }
+
if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
{
*has_cxx_zero_width_bf = true;
@@ -6403,8 +6418,13 @@ mips_return_in_msb (const_tree valtype)
tree fields[2];
bool has_cxx_zero_width_bf = false;
+
+ /* Its value is not used. */
+ bool has_cxx17_empty_base = false;
+
return (mips_fpr_return_fields (valtype, fields,
- &has_cxx_zero_width_bf) == 0
+ &has_cxx_zero_width_bf,
+ &has_cxx17_empty_base) == 0
|| has_cxx_zero_width_bf);
}
@@ -6501,11 +6521,18 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
mode = promote_function_mode (valtype, mode, &unsigned_p, func, 1);
bool has_cxx_zero_width_bf = false;
+ bool has_cxx17_empty_base = false;
int use_fpr = mips_fpr_return_fields (valtype, fields,
- &has_cxx_zero_width_bf);
+ &has_cxx_zero_width_bf,
+ &has_cxx17_empty_base);
+
+ /* If has_cxx_zero_width_bf and has_cxx17_empty_base are both
+ true, it *happens* that there is no ABI change. So we won't
+ inform in this case. */
if (TARGET_HARD_FLOAT
&& warn_psabi
&& has_cxx_zero_width_bf
+ && !has_cxx17_empty_base
&& use_fpr != 0)
{
static unsigned last_reported_type_uid;
@@ -6527,6 +6554,27 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
if (has_cxx_zero_width_bf)
use_fpr = 0;
+ if (TARGET_HARD_FLOAT
+ && warn_psabi
+ && use_fpr != 0
+ && has_cxx17_empty_base)
+ {
+ static unsigned last_reported_type_uid;
+ unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
+ if (uid != last_reported_type_uid)
+ {
+ static const char *url
+ = CHANGES_ROOT_URL
+ "gcc-12/changes.html#mips_cxx17_empty_bases";
+ inform (input_location,
+ "the ABI for returning a value with C++ 17 empty "
+ "bases but otherwise an aggregate with only one or "
+ "two floating-point fields was changed in GCC "
+ "%{12.1%}", url);
+ last_reported_type_uid = uid;
+ }
+ }
+
/* Handle structures whose fields are returned in $f0/$f2. */
switch (use_fpr)
{
diff --git a/gcc/testsuite/g++.target/mips/cxx17_empty_base.C b/gcc/testsuite/g++.target/mips/cxx17_empty_base.C
new file mode 100644
index 00000000000..da6d54b95d9
--- /dev/null
+++ b/gcc/testsuite/g++.target/mips/cxx17_empty_base.C
@@ -0,0 +1,20 @@
+// { dg-do compile }
+// { dg-options "-O2 -std=c++17 -mabi=64 -mhard-float" }
+// { dg-final { scan-assembler "\\\$f0" } }
+
+struct empty {};
+
+struct foo : empty
+{
+ double a;
+ double b;
+};
+
+struct foo
+make_foo(void) // { dg-message "the ABI for returning a value with C\\\+\\\+ 17 empty bases but otherwise an aggregate with only one or two floating-point fields was changed in GCC 12.1" }
+{
+ struct foo ret;
+ ret.a = 114;
+ ret.b = 514;
+ return ret;
+}
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64
2022-04-06 13:44 ` [PATCH v2] " Xi Ruoyao
@ 2022-04-06 14:34 ` Jakub Jelinek
2022-04-06 14:59 ` Xi Ruoyao
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2022-04-06 14:34 UTC (permalink / raw)
To: Xi Ruoyao; +Cc: Richard Sandiford, gcc-patches, Jakub Jelinek, YunQiang Su
On Wed, Apr 06, 2022 at 09:44:33PM +0800, Xi Ruoyao wrote:
> @@ -6527,6 +6554,27 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
> if (has_cxx_zero_width_bf)
> use_fpr = 0;
>
> + if (TARGET_HARD_FLOAT
> + && warn_psabi
> + && use_fpr != 0
> + && has_cxx17_empty_base)
> + {
> + static unsigned last_reported_type_uid;
> + unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
> + if (uid != last_reported_type_uid)
> + {
> + static const char *url
> + = CHANGES_ROOT_URL
> + "gcc-12/changes.html#mips_cxx17_empty_bases";
> + inform (input_location,
> + "the ABI for returning a value with C++ 17 empty "
Better write C++17 without space, that is what is used elsewhere too.
Ok with that change.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64
2022-04-06 14:34 ` Jakub Jelinek
@ 2022-04-06 14:59 ` Xi Ruoyao
0 siblings, 0 replies; 6+ messages in thread
From: Xi Ruoyao @ 2022-04-06 14:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Sandiford, gcc-patches, Jakub Jelinek, YunQiang Su
On Wed, 2022-04-06 at 16:34 +0200, Jakub Jelinek wrote:
> On Wed, Apr 06, 2022 at 09:44:33PM +0800, Xi Ruoyao wrote:
> > @@ -6527,6 +6554,27 @@ mips_function_value_1 (const_tree valtype,
> > const_tree fn_decl_or_type,
> > if (has_cxx_zero_width_bf)
> > use_fpr = 0;
> >
> > + if (TARGET_HARD_FLOAT
> > + && warn_psabi
> > + && use_fpr != 0
> > + && has_cxx17_empty_base)
> > + {
> > + static unsigned last_reported_type_uid;
> > + unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
> > + if (uid != last_reported_type_uid)
> > + {
> > + static const char *url
> > + = CHANGES_ROOT_URL
> > + "gcc-12/changes.html#mips_cxx17_empty_bases";
> > + inform (input_location,
> > + "the ABI for returning a value with C++ 17
> > empty "
>
> Better write C++17 without space, that is what is used elsewhere too.
> Ok with that change.
Pushed r12-8023 with the whitespace change.
--
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-06 14:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-06 12:33 [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64 Xi Ruoyao
2022-04-06 12:44 ` Jakub Jelinek
2022-04-06 12:48 ` Xi Ruoyao
2022-04-06 13:44 ` [PATCH v2] " Xi Ruoyao
2022-04-06 14:34 ` Jakub Jelinek
2022-04-06 14:59 ` Xi Ruoyao
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).