* [PATCH] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024]
@ 2022-03-31 16:13 Xi Ruoyao
2022-03-31 17:04 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Xi Ruoyao @ 2022-03-31 16:13 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek, Richard Sandiford, YunQiang Su
Part 1/2 of PR 102024 fix for MIPS.
No change for code generation. It only adds a -Wpsabi inform for return
values affected by the change in C++ FE.
The ABI is clear that any zero-width bit-field in a return value will
disallow using FPRs for it, so the behavior of GCC trunk is correct
(it's also same as clang).
gcc/
PR target/102024
* 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-1.C: New test.
---
gcc/config/mips/mips.cc | 51 ++++++++++++++++++++++--
gcc/testsuite/g++.target/mips/mips.exp | 34 ++++++++++++++++
gcc/testsuite/g++.target/mips/pr102024.C | 20 ++++++++++
3 files changed, 101 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.target/mips/mips.exp
create mode 100644 gcc/testsuite/g++.target/mips/pr102024.C
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index a1c4b437cd4..3284cf71f6f 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6274,10 +6274,17 @@ mips_callee_copies (cumulative_args_t, const function_arg_info &arg)
For n32 & n64, a structure with one or two fields is returned in
floating-point registers as long as every field has a floating-point
- type. */
+ type.
+
+ 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_ZERO_WIDTH_BF
+ to 1 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. */
static int
-mips_fpr_return_fields (const_tree valtype, tree *fields)
+mips_fpr_return_fields (const_tree valtype, tree *fields,
+ int *has_zero_width_bf)
{
tree field;
int i;
@@ -6294,6 +6301,12 @@ mips_fpr_return_fields (const_tree valtype, tree *fields)
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
+ {
+ *has_zero_width_bf = 1;
+ continue;
+ }
+
if (!SCALAR_FLOAT_TYPE_P (TREE_TYPE (field)))
return 0;
@@ -6319,11 +6332,14 @@ static bool
mips_return_in_msb (const_tree valtype)
{
tree fields[2];
+ int has_zero_width_bf = 0;
+ int use_fpr = mips_fpr_return_fields (valtype, fields,
+ &has_zero_width_bf);
return (TARGET_NEWABI
&& TARGET_BIG_ENDIAN
&& AGGREGATE_TYPE_P (valtype)
- && mips_fpr_return_fields (valtype, fields) == 0);
+ && (use_fpr == 0 || has_zero_width_bf));
}
/* Return true if the function return value MODE will get returned in a
@@ -6418,8 +6434,35 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
return values, promote the mode here too. */
mode = promote_function_mode (valtype, mode, &unsigned_p, func, 1);
+ int has_zero_width_bf = 0;
+ int use_fpr = mips_fpr_return_fields (valtype, fields,
+ &has_zero_width_bf);
+ if (TARGET_HARD_FLOAT &&
+ warn_psabi &&
+ has_zero_width_bf &&
+ use_fpr != 0)
+ {
+ 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#zero_width_bitfields";
+ inform (input_location,
+ "the ABI for returning a value containing "
+ "zero-width bit-fields but otherwise an aggregate "
+ "with only one or two floating-point fields was "
+ "retconned in GCC %{12.1%}", url);
+ last_reported_type_uid = uid;
+ }
+ }
+
+ if (has_zero_width_bf)
+ use_fpr = 0;
+
/* Handle structures whose fields are returned in $f0/$f2. */
- switch (mips_fpr_return_fields (valtype, fields))
+ switch (use_fpr)
{
case 1:
return mips_return_fpr_single (mode,
diff --git a/gcc/testsuite/g++.target/mips/mips.exp b/gcc/testsuite/g++.target/mips/mips.exp
new file mode 100644
index 00000000000..9fa7e771b4d
--- /dev/null
+++ b/gcc/testsuite/g++.target/mips/mips.exp
@@ -0,0 +1,34 @@
+# Copyright (C) 2019-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3. If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't a MIPS target.
+if ![istarget mips*-*-*] then {
+ return
+}
+
+# Load support procs.
+load_lib g++-dg.exp
+
+# Initialize `dg'.
+dg-init
+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] "" ""
+
+# All done.
+dg-finish
diff --git a/gcc/testsuite/g++.target/mips/pr102024.C b/gcc/testsuite/g++.target/mips/pr102024.C
new file mode 100644
index 00000000000..a4b99c79be0
--- /dev/null
+++ b/gcc/testsuite/g++.target/mips/pr102024.C
@@ -0,0 +1,20 @@
+// PR target/102024
+// { dg-do compile }
+// { dg-options "-O2 -mabi=64 -mhard-float" }
+// { dg-final { scan-assembler-not "\\\$f0" } }
+
+struct foo
+{
+ double a;
+ int : 0;
+ double b;
+};
+
+struct foo
+make_foo(void) // { dg-message "the ABI for returning a value containing zero-width bit-fields but otherwise an aggregate with only one or two floating-point fields was retconned in GCC 12.1" }
+{
+ struct foo ret;
+ ret.a = 114;
+ ret.b = 514;
+ return ret;
+}
--
2.35.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024]
2022-03-31 16:13 [PATCH] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024] Xi Ruoyao
@ 2022-03-31 17:04 ` Jakub Jelinek
2022-04-01 11:38 ` [PATCH v2] " Xi Ruoyao
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-03-31 17:04 UTC (permalink / raw)
To: Xi Ruoyao; +Cc: gcc-patches, Jakub Jelinek, Richard Sandiford, YunQiang Su
On Fri, Apr 01, 2022 at 12:13:30AM +0800, Xi Ruoyao wrote:
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -6274,10 +6274,17 @@ mips_callee_copies (cumulative_args_t, const function_arg_info &arg)
>
> For n32 & n64, a structure with one or two fields is returned in
> floating-point registers as long as every field has a floating-point
> - type. */
> + type.
> +
> + 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_ZERO_WIDTH_BF
> + to 1 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. */
>
> static int
> -mips_fpr_return_fields (const_tree valtype, tree *fields)
> +mips_fpr_return_fields (const_tree valtype, tree *fields,
> + int *has_zero_width_bf)
Use bool * and = true ?
> @@ -6319,11 +6332,14 @@ static bool
> mips_return_in_msb (const_tree valtype)
> {
> tree fields[2];
> + int has_zero_width_bf = 0;
I think it would be better to match more closely what the code did before.
if (!TARGET_NEWABI || !TARGET_BIG_ENDIAN || !AGGREGATE_TYPE_P (valtype))
return false;
tree fields[2];
bool has_zero_width_bf = false;
return (mips_fpr_return_fields (valtype, fields, &has_zero_width_bf) == 0
|| has_zero_width_bf);
> + int use_fpr = mips_fpr_return_fields (valtype, fields,
> + &has_zero_width_bf);
>
> return (TARGET_NEWABI
> && TARGET_BIG_ENDIAN
> && AGGREGATE_TYPE_P (valtype)
> - && mips_fpr_return_fields (valtype, fields) == 0);
> + && (use_fpr == 0 || has_zero_width_bf));
> }
>
> /* Return true if the function return value MODE will get returned in a
> @@ -6418,8 +6434,35 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
> return values, promote the mode here too. */
> mode = promote_function_mode (valtype, mode, &unsigned_p, func, 1);
>
> + int has_zero_width_bf = 0;
See above for int -> bool and 0 -> false.
> + int use_fpr = mips_fpr_return_fields (valtype, fields,
> + &has_zero_width_bf);
> + if (TARGET_HARD_FLOAT &&
> + warn_psabi &&
> + has_zero_width_bf &&
> + use_fpr != 0)
&&s need to go at the start of next line, not end of line.
> + {
> + static unsigned last_reported_type_uid;
> + unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
> + if (uid != last_reported_type_uid)
> + {
> + static const char *url =
Similarly for =
> + CHANGES_ROOT_URL
> + "gcc-12/changes.html#zero_width_bitfields";
> + inform (input_location,
> + "the ABI for returning a value containing "
> + "zero-width bit-fields but otherwise an aggregate "
> + "with only one or two floating-point fields was "
> + "retconned in GCC %{12.1%}", url);
In this diagnostics it is ok to talk about bit-fields as that is
the only thing that changes (compared to the other patch).
But again, was retconned -> changed ?
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024]
2022-03-31 17:04 ` Jakub Jelinek
@ 2022-04-01 11:38 ` Xi Ruoyao
2022-04-01 11:41 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Xi Ruoyao @ 2022-04-01 11:38 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Jakub Jelinek, Richard Sandiford, YunQiang Su
v1 -> v2:
* "int has_zero_width_bf" -> "bool has_cxx_zero_width_bf". "int" to
"bool" because the value is 0/1 only. Add "cxx" because it only
indicates C++ zero-width bit-fields (not those bit-fields from C).
* Coding style fix.
* Rewrite mips_return_in_msb so mips_fpr_return_fields is not called
unnecessarily.
* "retcon" -> "change".
gcc/
PR target/102024
* 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-1.C: New test.
---
gcc/config/mips/mips.cc | 58 ++++++++++++++++++++----
gcc/testsuite/g++.target/mips/mips.exp | 34 ++++++++++++++
gcc/testsuite/g++.target/mips/pr102024.C | 20 ++++++++
3 files changed, 104 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.target/mips/mips.exp
create mode 100644 gcc/testsuite/g++.target/mips/pr102024.C
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 91e1e964f94..83860b5d4b7 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6274,10 +6274,17 @@ mips_callee_copies (cumulative_args_t, const function_arg_info &arg)
For n32 & n64, a structure with one or two fields is returned in
floating-point registers as long as every field has a floating-point
- type. */
+ type.
+
+ 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. */
static int
-mips_fpr_return_fields (const_tree valtype, tree *fields)
+mips_fpr_return_fields (const_tree valtype, tree *fields,
+ bool *has_cxx_zero_width_bf)
{
tree field;
int i;
@@ -6294,6 +6301,12 @@ mips_fpr_return_fields (const_tree valtype, tree *fields)
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
+ {
+ *has_cxx_zero_width_bf = true;
+ continue;
+ }
+
if (!SCALAR_FLOAT_TYPE_P (TREE_TYPE (field)))
return 0;
@@ -6318,12 +6331,14 @@ mips_fpr_return_fields (const_tree valtype, tree *fields)
static bool
mips_return_in_msb (const_tree valtype)
{
- tree fields[2];
+ if (!TARGET_NEWABI || !TARGET_BIG_ENDIAN || !AGGREGATE_TYPE_P (valtype))
+ return false;
- return (TARGET_NEWABI
- && TARGET_BIG_ENDIAN
- && AGGREGATE_TYPE_P (valtype)
- && mips_fpr_return_fields (valtype, fields) == 0);
+ tree fields[2];
+ bool has_cxx_zero_width_bf = false;
+ return (mips_fpr_return_fields (valtype, fields,
+ &has_cxx_zero_width_bf) == 0
+ || has_cxx_zero_width_bf);
}
/* Return true if the function return value MODE will get returned in a
@@ -6418,8 +6433,35 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
return values, promote the mode here too. */
mode = promote_function_mode (valtype, mode, &unsigned_p, func, 1);
+ bool has_cxx_zero_width_bf = false;
+ int use_fpr = mips_fpr_return_fields (valtype, fields,
+ &has_cxx_zero_width_bf);
+ if (TARGET_HARD_FLOAT
+ && warn_psabi
+ && has_cxx_zero_width_bf
+ && use_fpr != 0)
+ {
+ 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#zero_width_bitfields";
+ inform (input_location,
+ "the ABI for returning a value containing "
+ "zero-width bit-fields but otherwise an aggregate "
+ "with only one or two floating-point fields was "
+ "changed in GCC %{12.1%}", url);
+ last_reported_type_uid = uid;
+ }
+ }
+
+ if (has_cxx_zero_width_bf)
+ use_fpr = 0;
+
/* Handle structures whose fields are returned in $f0/$f2. */
- switch (mips_fpr_return_fields (valtype, fields))
+ switch (use_fpr)
{
case 1:
return mips_return_fpr_single (mode,
diff --git a/gcc/testsuite/g++.target/mips/mips.exp b/gcc/testsuite/g++.target/mips/mips.exp
new file mode 100644
index 00000000000..9fa7e771b4d
--- /dev/null
+++ b/gcc/testsuite/g++.target/mips/mips.exp
@@ -0,0 +1,34 @@
+# Copyright (C) 2019-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3. If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't a MIPS target.
+if ![istarget mips*-*-*] then {
+ return
+}
+
+# Load support procs.
+load_lib g++-dg.exp
+
+# Initialize `dg'.
+dg-init
+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] "" ""
+
+# All done.
+dg-finish
diff --git a/gcc/testsuite/g++.target/mips/pr102024.C b/gcc/testsuite/g++.target/mips/pr102024.C
new file mode 100644
index 00000000000..331e6ca70e6
--- /dev/null
+++ b/gcc/testsuite/g++.target/mips/pr102024.C
@@ -0,0 +1,20 @@
+// PR target/102024
+// { dg-do compile }
+// { dg-options "-O2 -mabi=64 -mhard-float" }
+// { dg-final { scan-assembler-not "\\\$f0" } }
+
+struct foo
+{
+ double a;
+ int : 0;
+ double b;
+};
+
+struct foo
+make_foo(void) // { dg-message "the ABI for returning a value containing zero-width bit-fields 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] 4+ messages in thread
* Re: [PATCH v2] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024]
2022-04-01 11:38 ` [PATCH v2] " Xi Ruoyao
@ 2022-04-01 11:41 ` Jakub Jelinek
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2022-04-01 11:41 UTC (permalink / raw)
To: Xi Ruoyao; +Cc: gcc-patches, Jakub Jelinek, Richard Sandiford, YunQiang Su
On Fri, Apr 01, 2022 at 07:38:59PM +0800, Xi Ruoyao wrote:
> v1 -> v2:
>
> * "int has_zero_width_bf" -> "bool has_cxx_zero_width_bf". "int" to
> "bool" because the value is 0/1 only. Add "cxx" because it only
> indicates C++ zero-width bit-fields (not those bit-fields from C).
>
> * Coding style fix.
>
> * Rewrite mips_return_in_msb so mips_fpr_return_fields is not called
> unnecessarily.
>
> * "retcon" -> "change".
>
> gcc/
> PR target/102024
> * 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-1.C: New test.
LGTM, thanks.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-04-01 11:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 16:13 [PATCH] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024] Xi Ruoyao
2022-03-31 17:04 ` Jakub Jelinek
2022-04-01 11:38 ` [PATCH v2] " Xi Ruoyao
2022-04-01 11:41 ` Jakub Jelinek
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).