On Tue, 2021-09-21 at 17:35 -0500, Bill Schmidt wrote:
Hi!
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.
As noted in the PR, once the GCC 12 Changes page has text describing this issue,
we can update the diagnostic message to reference that URL. I'll handle that
in a follow-up patch.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
Is this okay for trunk?
How previously? is this one that will need all the backports?
Thanks!
Bill
2021-09-21 Bill Schmidt <wschm...@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.
ok
---
gcc/config/rs6000/rs6000-call.c | 39 ++++++++++++++++++---
gcc/testsuite/g++.target/powerpc/pr102024.C | 23 ++++++++++++
2 files changed, 57 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.target/powerpc/pr102024.C
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 7d485480225..c02b202b0cd 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -6227,7 +6227,7 @@ const struct altivec_builtin_types
altivec_overloaded_builtins[] = {
static int
rs6000_aggregate_candidate (const_tree type, machine_mode *modep,
- int *empty_base_seen)
+ int *empty_base_seen, int *zero_width_bf_seen)
{
machine_mode mode;
HOST_WIDE_INT size;
@@ -6298,7 +6298,8 @@ rs6000_aggregate_candidate (const_tree type, machine_mode
*modep,
return -1;
count = rs6000_aggregate_candidate (TREE_TYPE (type), modep,
- empty_base_seen);
+ empty_base_seen,
+ zero_width_bf_seen);
if (count == -1
|| !index
|| !TYPE_MAX_VALUE (index)
@@ -6336,6 +6337,12 @@ rs6000_aggregate_candidate (const_tree type,
machine_mode *modep,
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
+ {
+ *zero_width_bf_seen = 1;
+ continue;
+ }
+
Noting that the definition comes from tree.h and is
#define SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD(NODE, VAL) \
do { \
gcc_checking_assert (DECL_BIT_FIELD (NODE)); \
FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0 = (VAL); \
} while (0)
ok.
if (DECL_FIELD_ABI_IGNORED (field))
{
if (lookup_attribute ("no_unique_address",
@@ -6347,7 +6354,8 @@ rs6000_aggregate_candidate (const_tree type, machine_mode
*modep,
}
sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep,
- empty_base_seen);
+ empty_base_seen,
+ zero_width_bf_seen);
if (sub_count < 0)
return -1;
count += sub_count;
@@ -6381,7 +6389,8 @@ rs6000_aggregate_candidate (const_tree type, machine_mode
*modep,
continue;
sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep,
- empty_base_seen);
+ empty_base_seen,
+ zero_width_bf_seen);
if (sub_count < 0)
return -1;
count = count > sub_count ? count : sub_count;
@@ -6423,8 +6432,10 @@ rs6000_discover_homogeneous_aggregate (machine_mode
mode, const_tree type,
{
machine_mode field_mode = VOIDmode;
int empty_base_seen = 0;
+ int zero_width_bf_seen = 0;
int field_count = rs6000_aggregate_candidate (type, &field_mode,
- &empty_base_seen);
+ &empty_base_seen,
+ &zero_width_bf_seen);
That appears to be all of the callers of rs6000_aggregate_candidate.
(ok).
if (field_count > 0)
{
@@ -6460,6 +6471,24 @@ rs6000_discover_homogeneous_aggregate (machine_mode
mode, const_tree type,
last_reported_type_uid = uid;
}
}
+ if (zero_width_bf_seen && warn_psabi)
+ {
+ static unsigned last_reported_type_uid;
+ unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (type));
+ if (uid != last_reported_type_uid)
+ {
+ inform (input_location,
+ "parameter passing for an argument containing "
+ "zero-width bit fields but that is otherwise a "
+ "homogeneous aggregate changed in GCC 12.1");
+ last_reported_type_uid = uid;
+ }
+ if (elt_mode)
+ *elt_mode = mode;
+ if (n_elts)
+ *n_elts = 1;
+ return false;
+ }
In comparison to the other nearby warn_psabi logic, this seems
reasonable. (ok)
return true;
}
}
diff --git a/gcc/testsuite/g++.target/powerpc/pr102024.C
b/gcc/testsuite/g++.target/powerpc/pr102024.C
new file mode 100644
index 00000000000..29c9678acfd
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/pr102024.C
@@ -0,0 +1,23 @@
+// PR target/102024
+// { dg-do compile { target powerpc_elfv2 } }
+// { dg-options "-O2" }
+
+// Test that a zero-width bit field in an otherwise homogeneous aggregate
+// generates a psabi warning and passes arguments in GPRs.
+
+// { dg-final { scan-assembler-times {\mstd\M} 4 } }
+
+struct a_thing
+{
+ double x;
+ double y;
+ double z;
+ int : 0;
+ double w;
+};
+
+double
+foo (a_thing a) // { dg-message "parameter passing for an argument containing
zero-width bit fields but that is otherwise a homogeneous aggregate changed in GCC
12.1" }
+{
+ return a.x * a.y + a.z - a.w;
+}
lgtm
thanks
-Will
