On Tue, 31 Aug 2021, Richard Biener wrote:
> On Tue, 31 Aug 2021, Jakub Jelinek wrote:
>
> > Hi!
> >
> > The removal of remove_zero_width_bitfields function and its call from
> > C++ FE layout_class_type (which I've done in the P0466R5
> > layout-compatible helper intrinsics patch, so that the FE can actually
> > determine what is and isn't layout-compatible according to the spec)
> > unfortunately changed the ABI on various platforms.
> > The C FE has been keeping zero-width bitfields in the types, while
> > the C++ FE has been removing them after structure layout, so in various
> > cases when passing such structures in registers we had different ABI
> > between C and C++.
> >
> > The following patch doesn't change anything ABI-wise, but allows the
> > targets to decide what to do, emit -Wpsabi warnings etc.
> > Non-C zero width bitfields will be seen by the backends as normal
> > zero width bitfields, C++ zero width bitfields that used to be previously
> > removed will have DECL_FIELD_ABI_IGNORED flag set. It is ok to reuse
> > this flag, as it has been before used only on aggregate types and C++
> > bitfields are always scalar (and DECL_BIT_FIELD too).
> >
> > Each backend can then decide what it wants, whether it wants to keep
> > different ABI between C and C++ as in GCC 11 and older (for that it would
> > ignore for the homogenous aggregate decisions all DECL_FIELD_ABI_IGNORED
> > DECL_BIT_FIELD FIELD_DECLs), whether it wants to never ignore zero
> > width bitfields (no changes needed for that case, except perhaps -Wpsabi
> > warning should be added and for that DECL_FIELD_ABI_IGNORED can be tested),
> > or whether it wants to always ignore zero width bitfields (I think e.g.
> > riscv in GCC 10+ does that).
> >
> > All this patch does is set the flag and adjust backends so that nothing
> > changes for now.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Just to clarify - in the C++ FE these fields are meaningful for
> layout purposes but they are only supposed to influence layout
> but not ABI (but why does the C++ FE say that?) and thus the
> 'DECL_FIELD_ABI_IGNORED' is a good term to use? But we still want
> to have the backends decide whether to actually follow this advice
> and we do expect some to not do this?
Oh, and the tree.h docs for DECL_FIELD_ABI_IGNORED should be amended.
Richard.
> Thanks,
> Richard.
>
> > 2021-08-31 Jakub Jelinek <ja...@redhat.com>
> >
> > PR target/102024
> > gcc/
> > * config/arm/arm.c (aapcs_vfp_sub_candidate): Ignore
> > DECL_FIELD_ABI_IGNORED on DECL_BIT_FIELD fields.
> > * config/s390/s390.c (s390_function_arg_vector,
> > s390_function_arg_float): Likewise.
> > * config/ia64/ia64.c (hfa_element_mode): Likewise.
> > * config/aarch64/aarch64.c (aapcs_vfp_sub_candidate): Likewise.
> > * config/rs6000/rs6000.c (rs6000_special_adjust_field_align,
> > rs6000_special_round_type_align): Likewise.
> > * config/rs6000/rs6000-call.c (rs6000_aggregate_candidate): Likewise.
> > gcc/cp/
> > * class.c (layout_class_type): Set DECL_FIELD_ABI_IGNORED on zero
> > width bitfields we used to remove in GCC 11 and earlier.
> >
> > --- gcc/config/arm/arm.c.jj 2021-08-30 08:36:11.027519310 +0200
> > +++ gcc/config/arm/arm.c 2021-08-30 13:36:57.068845279 +0200
> > @@ -6332,7 +6332,7 @@ aapcs_vfp_sub_candidate (const_tree type
> > if (TREE_CODE (field) != FIELD_DECL)
> > continue;
> >
> > - if (DECL_FIELD_ABI_IGNORED (field))
> > + if (DECL_FIELD_ABI_IGNORED (field) && !DECL_BIT_FIELD (field))
> > {
> > /* See whether this is something that earlier versions of
> > GCC failed to ignore. */
> > --- gcc/config/s390/s390.c.jj 2021-08-05 10:26:15.611554712 +0200
> > +++ gcc/config/s390/s390.c 2021-08-30 13:38:29.493548311 +0200
> > @@ -12167,7 +12167,7 @@ s390_function_arg_vector (machine_mode m
> > if (TREE_CODE (field) != FIELD_DECL)
> > continue;
> >
> > - if (DECL_FIELD_ABI_IGNORED (field))
> > + if (DECL_FIELD_ABI_IGNORED (field) && !DECL_BIT_FIELD (field))
> > {
> > if (lookup_attribute ("no_unique_address",
> > DECL_ATTRIBUTES (field)))
> > @@ -12251,7 +12251,7 @@ s390_function_arg_float (machine_mode mo
> > {
> > if (TREE_CODE (field) != FIELD_DECL)
> > continue;
> > - if (DECL_FIELD_ABI_IGNORED (field))
> > + if (DECL_FIELD_ABI_IGNORED (field) && !DECL_BIT_FIELD (field))
> > {
> > if (lookup_attribute ("no_unique_address",
> > DECL_ATTRIBUTES (field)))
> > --- gcc/config/ia64/ia64.c.jj 2021-05-18 14:26:10.193220099 +0200
> > +++ gcc/config/ia64/ia64.c 2021-08-30 13:37:32.855343096 +0200
> > @@ -4665,7 +4665,8 @@ hfa_element_mode (const_tree type, bool
> > case QUAL_UNION_TYPE:
> > for (t = TYPE_FIELDS (type); t; t = DECL_CHAIN (t))
> > {
> > - if (TREE_CODE (t) != FIELD_DECL || DECL_FIELD_ABI_IGNORED (t))
> > + if (TREE_CODE (t) != FIELD_DECL
> > + || (DECL_FIELD_ABI_IGNORED (t) && !DECL_BIT_FIELD (t)))
> > continue;
> >
> > mode = hfa_element_mode (TREE_TYPE (t), 1);
> > --- gcc/config/aarch64/aarch64.c.jj 2021-08-17 13:58:10.652245152 +0200
> > +++ gcc/config/aarch64/aarch64.c 2021-08-30 13:36:28.268249427 +0200
> > @@ -19019,7 +19019,7 @@ aapcs_vfp_sub_candidate (const_tree type
> > if (TREE_CODE (field) != FIELD_DECL)
> > continue;
> >
> > - if (DECL_FIELD_ABI_IGNORED (field))
> > + if (DECL_FIELD_ABI_IGNORED (field) && !DECL_BIT_FIELD (field))
> > {
> > /* See whether this is something that earlier versions of
> > GCC failed to ignore. */
> > --- gcc/config/rs6000/rs6000.c.jj 2021-08-30 08:36:11.110518142 +0200
> > +++ gcc/config/rs6000/rs6000.c 2021-08-30 13:39:42.790519755 +0200
> > @@ -8023,7 +8023,8 @@ rs6000_special_adjust_field_align (tree
> > /* Skip all non field decls */
> > while (field != NULL
> > && (TREE_CODE (field) != FIELD_DECL
> > - || DECL_FIELD_ABI_IGNORED (field)))
> > + || (DECL_FIELD_ABI_IGNORED (field)
> > + && !DECL_BIT_FIELD (field))))
> > field = DECL_CHAIN (field);
> >
> > if (! field)
> > @@ -8068,7 +8069,8 @@ rs6000_special_round_type_align (tree ty
> > /* Skip all non field decls */
> > while (field != NULL
> > && (TREE_CODE (field) != FIELD_DECL
> > - || DECL_FIELD_ABI_IGNORED (field)))
> > + || (DECL_FIELD_ABI_IGNORED (field)
> > + && !DECL_BIT_FIELD (field))))
> > field = DECL_CHAIN (field);
> >
> > if (! field)
> > @@ -8110,7 +8112,8 @@ darwin_rs6000_special_round_type_align (
> > /* Skip all non field decls */
> > while (field != NULL
> > && (TREE_CODE (field) != FIELD_DECL
> > - || DECL_FIELD_ABI_IGNORED (field)))
> > + || (DECL_FIELD_ABI_IGNORED (field)
> > + && !DECL_BIT_FIELD (field))))
> > field = DECL_CHAIN (field);
> > if (! field)
> > break;
> > --- gcc/config/rs6000/rs6000-call.c.jj 2021-08-30 08:36:11.106518198
> > +0200
> > +++ gcc/config/rs6000/rs6000-call.c 2021-08-30 13:40:02.261246527 +0200
> > @@ -6335,7 +6335,7 @@ rs6000_aggregate_candidate (const_tree t
> > if (TREE_CODE (field) != FIELD_DECL)
> > continue;
> >
> > - if (DECL_FIELD_ABI_IGNORED (field))
> > + if (DECL_FIELD_ABI_IGNORED (field) && !DECL_BIT_FIELD (field))
> > {
> > if (lookup_attribute ("no_unique_address",
> > DECL_ATTRIBUTES (field)))
> > --- gcc/cp/class.c.jj 2021-08-19 11:42:27.269423736 +0200
> > +++ gcc/cp/class.c 2021-08-30 12:43:43.371658433 +0200
> > @@ -6744,6 +6744,22 @@ layout_class_type (tree t, tree *virtual
> > normalize_rli (rli);
> > }
> >
> > + /* We used to remove zero width bitfields at this point, while the C FE
> > + never did that. That caused ABI differences on various targets.
> > + Set the DECL_FIELD_ABI_IGNORED flag on them instead, so that the
> > backends
> > + can emit -Wpsabi warnings in the cases where the ABI changed. */
> > + for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
> > + if (TREE_CODE (field) == FIELD_DECL
> > + && DECL_C_BIT_FIELD (field)
> > + /* We should not be confused by the fact that grokbitfield
> > + temporarily sets the width of the bit field into
> > + DECL_BIT_FIELD_REPRESENTATIVE (field).
> > + check_bitfield_decl eventually sets DECL_SIZE (field)
> > + to that width. */
> > + && (DECL_SIZE (field) == NULL_TREE
> > + || integer_zerop (DECL_SIZE (field))))
> > + DECL_FIELD_ABI_IGNORED (field) = 1;
> > +
> > if (CLASSTYPE_NON_LAYOUT_POD_P (t) || CLASSTYPE_EMPTY_P (t))
> > {
> > /* T needs a different layout as a base (eliding virtual bases
> >
> > Jakub
> >
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
