| Message ID | 20210831074430.GS920497@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++, abi: Set DECL_FIELD_ABI_IGNORED on C++ zero width bitfields [PR102024] | expand |
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? Thanks, Richard. > 2021-08-31 Jakub Jelinek <jakub@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 > >
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 <jakub@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 > > > > > >
On Tue, Aug 31, 2021 at 09:57:44AM +0200, Richard Biener wrote: > 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? The removal of zero-width bitfields was added (after structure layout) by https://gcc.gnu.org/legacy-ml/gcc-patches/1999-12/msg00589.html https://gcc.gnu.org/legacy-ml/gcc-patches/1999-12/msg00641.html The comment about it was: /* Delete all zero-width bit-fields from the list of fields. Now that we have layed out the type they are no longer important. */ The only spot I see zero-width bit-fields mentioned in the Itanium ABI is: empty class A class with no non-static data members other than empty data members, no unnamed bit-fields other than zero-width bit-fields, no virtual functions, no virtual base classes, and no non-empty non-virtual proper base classes. nearly empty class A class that contains a virtual pointer, but no other data except (possibly) virtual bases. In particular, it: - has no non-static data members and no non-zero-width unnamed bit-fields, - has no direct base classes that are not either empty, nearly empty, or virtual, - has at most one non-virtual, nearly empty direct base class, and - has no proper base class that is empty, not morally virtual, and at an offset other than zero. Such classes may be primary base classes even if virtual, sharing a virtual pointer with the derived class. and the removal of remove_zero_width_bit_fields I believe didn't change anything on that, e.g. is_empty_class uses CLASSTYPE_EMPTY_P flag whose computation takes: if (DECL_C_BIT_FIELD (field) && integer_zerop (DECL_BIT_FIELD_REPRESENTATIVE (field))) /* We don't treat zero-width bitfields as making a class non-empty. */ ; into account (that is still before the bit-fields are finalized so width is stored differently, and it is necessary before the former remove_zero_width_bit_fields call). The flag for these zero-width bitfields is a good name for the case where a target decides to keep the old GCC 11 ABI of not ignoring them for C and ignoring them for C++, in other cases it can be a little bit confusing, but I think we could define another macro with the same value for it if we find a good name for it (dunno what it would be though). But even if we have another name, if we reuse the flag we need to take it into account in the target code, and using a different flag would be a waste of the precious bits. Perhaps just clarify in tree.h above the DECL_FIELD_ABI_IGNORED the cases in which it is set? Jakub
On Tue, 31 Aug 2021, Jakub Jelinek wrote: > On Tue, Aug 31, 2021 at 09:57:44AM +0200, Richard Biener wrote: > > 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? > > The removal of zero-width bitfields was added (after structure layout) > by > https://gcc.gnu.org/legacy-ml/gcc-patches/1999-12/msg00589.html > https://gcc.gnu.org/legacy-ml/gcc-patches/1999-12/msg00641.html > The comment about it was: > /* Delete all zero-width bit-fields from the list of fields. Now > that we have layed out the type they are no longer important. */ > The only spot I see zero-width bit-fields mentioned in the Itanium ABI is: > > empty class > A class with no non-static data members other than empty data members, > no unnamed bit-fields other than zero-width bit-fields, no virtual functions, > no virtual base classes, and no non-empty non-virtual proper base classes. > > nearly empty class > A class that contains a virtual pointer, but no other data except (possibly) virtual bases. In particular, it: > - has no non-static data members and no non-zero-width unnamed bit-fields, > - has no direct base classes that are not either empty, nearly empty, or virtual, > - has at most one non-virtual, nearly empty direct base class, and > - has no proper base class that is empty, not morally virtual, and at an offset other than zero. > Such classes may be primary base classes even if virtual, sharing a virtual pointer with the derived class. > > and the removal of remove_zero_width_bit_fields I believe didn't change > anything on that, e.g. is_empty_class uses CLASSTYPE_EMPTY_P flag whose > computation takes: > if (DECL_C_BIT_FIELD (field) > && integer_zerop (DECL_BIT_FIELD_REPRESENTATIVE (field))) > /* We don't treat zero-width bitfields as making a class > non-empty. */ > ; > into account (that is still before the bit-fields are finalized so > width is stored differently, and it is necessary before the > former remove_zero_width_bit_fields call). > > The flag for these zero-width bitfields is a good name for the case > where a target decides to keep the old GCC 11 ABI of not ignoring them > for C and ignoring them for C++, in other cases it can be a little bit > confusing, but I think we could define another macro with the same > value for it if we find a good name for it (dunno what it would be though). > But even if we have another name, if we reuse the flag we need to take > it into account in the target code, and using a different flag would be a > waste of the precious bits. > Perhaps just clarify in tree.h above the DECL_FIELD_ABI_IGNORED the cases > in which it is set? Yeah, I think it conflates the C++ [Itanium] ABI and the psABI for calling conventions. The 'ABI' in DECL_FIELD_ABI_IGNORED refers to the psABI as far as I understand the situation, but then it might still be important for the psABI when dealing with (non-)homogenous aggregates ... So _maybe_ DECL_FIELD_FOR_LAYOUT might capture the bits better - the field is present for layout (and possibly ABI), but it doesn't carry any data so it doesn't have to be passed across function boundary for example. Anyway, I'm not stuck to whatever naming we choose but the situation is complicated enough that we want some more elaborate docs in tree.h I'll leave the final ACK to Jason (unless he's on vacation). Thanks, Richard.
On 8/31/21 5:15 AM, Richard Biener wrote: > On Tue, 31 Aug 2021, Jakub Jelinek wrote: > >> On Tue, Aug 31, 2021 at 09:57:44AM +0200, Richard Biener wrote: >>> 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?) The code to remove zero-length bit-fields was copied from the C front end when G++ was first created. It was removed from the C front end by Joseph's r84279. The last thread for that patch is https://gcc.gnu.org/pipermail/gcc-patches/2004-July/thread.html#142984 The patch and that thread contain no discussion of zero-length bit-fields, nor is there one in any of the testcases added by the patch. I guess the assumption both before and after that patch was that zero-length bit-fields would not affect ABI, whether or not they appeared in TYPE_FIELDS. Are the targets where ABI is affected all new since then? >>> 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? >> >> The removal of zero-width bitfields was added (after structure layout) >> by >> https://gcc.gnu.org/legacy-ml/gcc-patches/1999-12/msg00589.html >> https://gcc.gnu.org/legacy-ml/gcc-patches/1999-12/msg00641.html >> The comment about it was: >> /* Delete all zero-width bit-fields from the list of fields. Now >> that we have layed out the type they are no longer important. */ >> The only spot I see zero-width bit-fields mentioned in the Itanium ABI is: >> >> empty class >> A class with no non-static data members other than empty data members, >> no unnamed bit-fields other than zero-width bit-fields, no virtual functions, >> no virtual base classes, and no non-empty non-virtual proper base classes. >> >> nearly empty class >> A class that contains a virtual pointer, but no other data except (possibly) virtual bases. In particular, it: >> - has no non-static data members and no non-zero-width unnamed bit-fields, >> - has no direct base classes that are not either empty, nearly empty, or virtual, >> - has at most one non-virtual, nearly empty direct base class, and >> - has no proper base class that is empty, not morally virtual, and at an offset other than zero. >> Such classes may be primary base classes even if virtual, sharing a virtual pointer with the derived class. >> >> and the removal of remove_zero_width_bit_fields I believe didn't change >> anything on that, e.g. is_empty_class uses CLASSTYPE_EMPTY_P flag whose >> computation takes: >> if (DECL_C_BIT_FIELD (field) >> && integer_zerop (DECL_BIT_FIELD_REPRESENTATIVE (field))) >> /* We don't treat zero-width bitfields as making a class >> non-empty. */ >> ; >> into account (that is still before the bit-fields are finalized so >> width is stored differently, and it is necessary before the >> former remove_zero_width_bit_fields call). >> >> The flag for these zero-width bitfields is a good name for the case >> where a target decides to keep the old GCC 11 ABI of not ignoring them >> for C and ignoring them for C++, in other cases it can be a little bit >> confusing, but I think we could define another macro with the same >> value for it if we find a good name for it (dunno what it would be though). >> But even if we have another name, if we reuse the flag we need to take >> it into account in the target code, and using a different flag would be a >> waste of the precious bits. >> Perhaps just clarify in tree.h above the DECL_FIELD_ABI_IGNORED the cases >> in which it is set? > > Yeah, I think it conflates the C++ [Itanium] ABI and the psABI for > calling conventions. The 'ABI' in DECL_FIELD_ABI_IGNORED refers > to the psABI as far as I understand the situation, but then it > might still be important for the psABI when dealing with > (non-)homogenous aggregates ... > > So _maybe_ DECL_FIELD_FOR_LAYOUT might capture the bits better - the > field is present for layout (and possibly ABI), but it doesn't carry > any data so it doesn't have to be passed across function boundary > for example. > > Anyway, I'm not stuck to whatever naming we choose but the situation > is complicated enough that we want some more elaborate docs in tree.h > I'll leave the final ACK to Jason (unless he's on vacation). DECL_FIELD_FOR_LAYOUT sounds better to me. Jason
On Wed, Sep 01, 2021 at 05:32:07PM -0400, Jason Merrill wrote: > On 8/31/21 5:15 AM, Richard Biener wrote: > > On Tue, 31 Aug 2021, Jakub Jelinek wrote: > > > > > On Tue, Aug 31, 2021 at 09:57:44AM +0200, Richard Biener wrote: > > > > 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?) > > The code to remove zero-length bit-fields was copied from the C front end > when G++ was first created. It was removed from the C front end by Joseph's > r84279. The last thread for that patch is > > https://gcc.gnu.org/pipermail/gcc-patches/2004-July/thread.html#142984 > > The patch and that thread contain no discussion of zero-length bit-fields, > nor is there one in any of the testcases added by the patch. > > I guess the assumption both before and after that patch was that zero-length > bit-fields would not affect ABI, whether or not they appeared in > TYPE_FIELDS. Are the targets where ABI is affected all new since then? Ah, thanks for the archeology. So it indeed seems like in theory an ABI change between GCC 3.4 and 4.0 for C then on some of the targets like x86_64 which already existed in 3.2-ish era. I actually couldn't see a difference between C and C++ in that era on x86_64, e.g. 3.3 C and C++ both work as C and C++ now, as if the zero width bitfields aren't removed. Before the PR42217 fix the C++ FE wasn't really removing the zero width bitfields properly, so it is actually 4.5/4.4-ish regression for C++. > > Anyway, I'm not stuck to whatever naming we choose but the situation > > is complicated enough that we want some more elaborate docs in tree.h > > I'll leave the final ACK to Jason (unless he's on vacation). > > DECL_FIELD_FOR_LAYOUT sounds better to me. Just for the zero-width bitfields, or also for the class FIELD_DECLs on which we set DECL_FIELD_ABI_IGNORED now? I could e.g. keep DECL_FIELD_ABI_IGNORED for the class ones, by making #define DECL_FIELD_ABI_IGNORED(field) \ (!DECL_BIT_FIELD (field) && (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0)) add SET_DECL_FIELD_ABI_IGNORED macro and similarly define DECL_FIELD_FOR_LAYOUT and SET_DECL_FIELD_FOR_LAYOUT (just it would do DECL_BIT_FIELD (field) check in that case). Jakub
On Thu, Sep 02, 2021 at 12:19:03AM +0200, Jakub Jelinek via Gcc-patches wrote: > Ah, thanks for the archeology. So it indeed seems like in theory an ABI change > between GCC 3.4 and 4.0 for C then on some of the targets like x86_64 which > already existed in 3.2-ish era. I actually couldn't see a difference > between C and C++ in that era on x86_64, e.g. 3.3 C and C++ both work as > C and C++ now, as if the zero width bitfields aren't removed. > Before the PR42217 fix the C++ FE wasn't really removing the zero width bitfields > properly, so it is actually 4.5/4.4-ish regression for C++. Ok, verified even the C FE used to suffer from the same issue as PR42217 and didn't actually ever remove any zero width bitfields, while grokfield put the field width into DECL_INITIAL, then finish_struct did: for (x = fieldlist; x; x = TREE_CHAIN (x)) { ... if (DECL_INITIAL (x)) { unsigned HOST_WIDE_INT width = tree_low_cst (DECL_INITIAL (x), 1); DECL_SIZE (x) = bitsize_int (width); DECL_BIT_FIELD (x) = 1; SET_DECL_C_BIT_FIELD (x); } DECL_INITIAL (x) = 0; ... } and only a few lines later it did: /* Delete all zero-width bit-fields from the fieldlist. */ { tree *fieldlistp = &fieldlist; while (*fieldlistp) if (TREE_CODE (*fieldlistp) == FIELD_DECL && DECL_INITIAL (*fieldlistp)) *fieldlistp = TREE_CHAIN (*fieldlistp); else fieldlistp = &TREE_CHAIN (*fieldlistp); } but DECL_INITIAL was already guaranteed to be NULL here. PR42217 actually was the same problem as PR102019, but was fixed by actually making the zero-width bit-field removal work when it never worked before. Here is an updated patch that instead uses separate macros for the previous DECL_FIELD_ABI_IGNORED meaning and for the C++ zero-width bitfields. The backends don't need any changes in that case (until they want to actually use the new macro for the -Wpsabi or ABI decisions): 2021-09-02 Jakub Jelinek <jakub@redhat.com> PR target/102024 gcc/ * tree.h (DECL_FIELD_ABI_IGNORED): Changed into rvalue only macro that is false if DECL_BIT_FIELD. (SET_DECL_FIELD_ABI_IGNORED, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD, SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD): Define. * tree-streamer-out.c (pack_ts_decl_common_value_fields): For DECL_BIT_FIELD stream DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead of DECL_FIELD_ABI_IGNORED. * tree-streamer-in.c (unpack_ts_decl_common_value_fields): Use SET_DECL_FIELD_ABI_IGNORED instead of writing to DECL_FIELD_ABI_IGNORED and for DECL_BIT_FIELD use SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead. * lto-streamer-out.c (hash_tree): For DECL_BIT_FIELD hash DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead of DECL_FIELD_ABI_IGNORED. gcc/cp/ * class.c (build_base_field): Use SET_DECL_FIELD_ABI_IGNORED instead of writing to DECL_FIELD_ABI_IGNORED. (layout_class_type): Likewise. In the place where zero-width bitfields used to be removed, use SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on those fields instead. gcc/lto/ * lto-common.c (compare_tree_sccs_1): Also compare DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD values. --- gcc/tree.h.jj 2021-09-01 21:30:30.551306387 +0200 +++ gcc/tree.h 2021-09-02 10:34:43.559851006 +0200 @@ -2852,16 +2852,34 @@ extern void decl_value_expr_insert (tree /* In a FIELD_DECL, indicates this field should be bit-packed. */ #define DECL_PACKED(NODE) (FIELD_DECL_CHECK (NODE)->base.u.bits.packed_flag) +/* Nonzero in a FIELD_DECL means it is a bit field, and must be accessed + specially. */ +#define DECL_BIT_FIELD(NODE) (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_1) + /* In a FIELD_DECL, indicates this field should be ignored for ABI decisions like passing/returning containing struct by value. Set for C++17 empty base artificial FIELD_DECLs as well as empty [[no_unique_address]] non-static data members. */ #define DECL_FIELD_ABI_IGNORED(NODE) \ - (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0) + (!DECL_BIT_FIELD (NODE) && (NODE)->decl_common.decl_flag_0) +#define SET_DECL_FIELD_ABI_IGNORED(NODE, VAL) \ + do { \ + gcc_checking_assert (!DECL_BIT_FIELD (NODE)); \ + FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0 = (VAL); \ + } while (0) -/* Nonzero in a FIELD_DECL means it is a bit field, and must be accessed - specially. */ -#define DECL_BIT_FIELD(NODE) (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_1) +/* In a FIELD_DECL, indicates C++ zero-width bitfield that used to be + removed from the IL since PR42217 until PR101539 and by that changed + the ABI on several targets. This flag is provided so that the backends + can decide on the ABI with zero-width bitfields and emit -Wpsabi + warnings. */ +#define DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD(NODE) \ + (DECL_BIT_FIELD (NODE) && (NODE)->decl_common.decl_flag_0) +#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) /* Used in a FIELD_DECL to indicate that we cannot form the address of this component. This makes it possible for Type-Based Alias Analysis --- gcc/tree-streamer-out.c.jj 2021-06-02 10:08:14.381447116 +0200 +++ gcc/tree-streamer-out.c 2021-09-02 10:34:58.377638627 +0200 @@ -219,7 +219,10 @@ pack_ts_decl_common_value_fields (struct bp_pack_value (bp, DECL_PACKED (expr), 1); bp_pack_value (bp, DECL_NONADDRESSABLE_P (expr), 1); bp_pack_value (bp, DECL_PADDING_P (expr), 1); - bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1); + if (DECL_BIT_FIELD (expr)) + bp_pack_value (bp, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (expr), 1); + else + bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1); bp_pack_value (bp, expr->decl_common.off_align, 8); } --- gcc/tree-streamer-in.c.jj 2021-06-02 10:08:14.372447243 +0200 +++ gcc/tree-streamer-in.c 2021-09-02 10:35:28.635204959 +0200 @@ -256,7 +256,11 @@ unpack_ts_decl_common_value_fields (stru DECL_PACKED (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_NONADDRESSABLE_P (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_PADDING_P (expr) = (unsigned) bp_unpack_value (bp, 1); - DECL_FIELD_ABI_IGNORED (expr) = (unsigned) bp_unpack_value (bp, 1); + unsigned val = (unsigned) bp_unpack_value (bp, 1); + if (DECL_BIT_FIELD (expr)) + SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (expr, val); + else + SET_DECL_FIELD_ABI_IGNORED (expr, val); expr->decl_common.off_align = bp_unpack_value (bp, 8); } --- gcc/lto-streamer-out.c.jj 2021-02-03 13:36:17.704980676 +0100 +++ gcc/lto-streamer-out.c 2021-09-02 10:36:34.796256706 +0200 @@ -1271,7 +1271,10 @@ hash_tree (struct streamer_tree_cache_d hstate.add_flag (DECL_PACKED (t)); hstate.add_flag (DECL_NONADDRESSABLE_P (t)); hstate.add_flag (DECL_PADDING_P (t)); - hstate.add_flag (DECL_FIELD_ABI_IGNORED (t)); + if (DECL_BIT_FIELD (t)) + hstate.add_flag (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (t)); + else + hstate.add_flag (DECL_FIELD_ABI_IGNORED (t)); hstate.add_int (DECL_OFFSET_ALIGN (t)); } else if (code == VAR_DECL) --- gcc/cp/class.c.jj 2021-09-01 21:30:30.473307484 +0200 +++ gcc/cp/class.c 2021-09-02 10:36:16.860513770 +0200 @@ -4634,7 +4634,7 @@ build_base_field (record_layout_info rli DECL_FIELD_OFFSET (decl) = BINFO_OFFSET (binfo); DECL_FIELD_BIT_OFFSET (decl) = bitsize_zero_node; SET_DECL_OFFSET_ALIGN (decl, BITS_PER_UNIT); - DECL_FIELD_ABI_IGNORED (decl) = 1; + SET_DECL_FIELD_ABI_IGNORED (decl, 1); } /* An empty virtual base causes a class to be non-empty @@ -6658,7 +6658,7 @@ layout_class_type (tree t, tree *virtual } else if (might_overlap && is_empty_class (type)) { - DECL_FIELD_ABI_IGNORED (field) = 1; + SET_DECL_FIELD_ABI_IGNORED (field, 1); layout_empty_base_or_field (rli, field, empty_base_offsets); } else @@ -6746,6 +6746,23 @@ layout_class_type (tree t, tree *virtual normalize_rli (rli); } + /* We used to remove zero width bitfields at this point since PR42217, + while the C FE never did that. That caused ABI differences on various + targets. Set the DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD 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)))) + SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field, 1); + if (CLASSTYPE_NON_LAYOUT_POD_P (t) || CLASSTYPE_EMPTY_P (t)) { /* T needs a different layout as a base (eliding virtual bases --- gcc/lto/lto-common.c.jj 2021-06-02 10:08:14.347447598 +0200 +++ gcc/lto/lto-common.c 2021-09-02 10:35:49.480906189 +0200 @@ -1187,6 +1187,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t compare_values (DECL_NONADDRESSABLE_P); compare_values (DECL_PADDING_P); compare_values (DECL_FIELD_ABI_IGNORED); + compare_values (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD); compare_values (DECL_OFFSET_ALIGN); } else if (code == VAR_DECL) Jakub
On 9/2/21 4:49 AM, Jakub Jelinek wrote: > On Thu, Sep 02, 2021 at 12:19:03AM +0200, Jakub Jelinek via Gcc-patches wrote: >> Ah, thanks for the archeology. So it indeed seems like in theory an ABI change >> between GCC 3.4 and 4.0 for C then on some of the targets like x86_64 which >> already existed in 3.2-ish era. I actually couldn't see a difference >> between C and C++ in that era on x86_64, e.g. 3.3 C and C++ both work as >> C and C++ now, as if the zero width bitfields aren't removed. >> Before the PR42217 fix the C++ FE wasn't really removing the zero width bitfields >> properly, so it is actually 4.5/4.4-ish regression for C++. > > Ok, verified even the C FE used to suffer from the same issue as PR42217 and > didn't actually ever remove any zero width bitfields, while grokfield put > the field width into DECL_INITIAL, then finish_struct did: > for (x = fieldlist; x; x = TREE_CHAIN (x)) > { > ... > if (DECL_INITIAL (x)) > { > unsigned HOST_WIDE_INT width = tree_low_cst (DECL_INITIAL (x), 1); > DECL_SIZE (x) = bitsize_int (width); > DECL_BIT_FIELD (x) = 1; > SET_DECL_C_BIT_FIELD (x); > } > > DECL_INITIAL (x) = 0; > ... > } > and only a few lines later it did: > /* Delete all zero-width bit-fields from the fieldlist. */ > { > tree *fieldlistp = &fieldlist; > while (*fieldlistp) > if (TREE_CODE (*fieldlistp) == FIELD_DECL && DECL_INITIAL (*fieldlistp)) > *fieldlistp = TREE_CHAIN (*fieldlistp); > else > fieldlistp = &TREE_CHAIN (*fieldlistp); > } > but DECL_INITIAL was already guaranteed to be NULL here. PR42217 actually > was the same problem as PR102019, but was fixed by actually making the > zero-width bit-field removal work when it never worked before. Ah, oops :/ > Here is an updated patch that instead uses separate macros for the > previous DECL_FIELD_ABI_IGNORED meaning and for the C++ zero-width > bitfields. The backends don't need any changes in that case (until they > want to actually use the new macro for the -Wpsabi or ABI decisions): LGTM. > 2021-09-02 Jakub Jelinek <jakub@redhat.com> > > PR target/102024 > gcc/ > * tree.h (DECL_FIELD_ABI_IGNORED): Changed into rvalue only macro > that is false if DECL_BIT_FIELD. > (SET_DECL_FIELD_ABI_IGNORED, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD, > SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD): Define. > * tree-streamer-out.c (pack_ts_decl_common_value_fields): For > DECL_BIT_FIELD stream DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead > of DECL_FIELD_ABI_IGNORED. > * tree-streamer-in.c (unpack_ts_decl_common_value_fields): Use > SET_DECL_FIELD_ABI_IGNORED instead of writing to > DECL_FIELD_ABI_IGNORED and for DECL_BIT_FIELD use > SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead. > * lto-streamer-out.c (hash_tree): For DECL_BIT_FIELD hash > DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead of DECL_FIELD_ABI_IGNORED. > gcc/cp/ > * class.c (build_base_field): Use SET_DECL_FIELD_ABI_IGNORED > instead of writing to DECL_FIELD_ABI_IGNORED. > (layout_class_type): Likewise. In the place where zero-width > bitfields used to be removed, use > SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on those fields instead. > gcc/lto/ > * lto-common.c (compare_tree_sccs_1): Also compare > DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD values. > > --- gcc/tree.h.jj 2021-09-01 21:30:30.551306387 +0200 > +++ gcc/tree.h 2021-09-02 10:34:43.559851006 +0200 > @@ -2852,16 +2852,34 @@ extern void decl_value_expr_insert (tree > /* In a FIELD_DECL, indicates this field should be bit-packed. */ > #define DECL_PACKED(NODE) (FIELD_DECL_CHECK (NODE)->base.u.bits.packed_flag) > > +/* Nonzero in a FIELD_DECL means it is a bit field, and must be accessed > + specially. */ > +#define DECL_BIT_FIELD(NODE) (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_1) > + > /* In a FIELD_DECL, indicates this field should be ignored for ABI decisions > like passing/returning containing struct by value. > Set for C++17 empty base artificial FIELD_DECLs as well as > empty [[no_unique_address]] non-static data members. */ > #define DECL_FIELD_ABI_IGNORED(NODE) \ > - (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0) > + (!DECL_BIT_FIELD (NODE) && (NODE)->decl_common.decl_flag_0) > +#define SET_DECL_FIELD_ABI_IGNORED(NODE, VAL) \ > + do { \ > + gcc_checking_assert (!DECL_BIT_FIELD (NODE)); \ > + FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0 = (VAL); \ > + } while (0) > > -/* Nonzero in a FIELD_DECL means it is a bit field, and must be accessed > - specially. */ > -#define DECL_BIT_FIELD(NODE) (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_1) > +/* In a FIELD_DECL, indicates C++ zero-width bitfield that used to be > + removed from the IL since PR42217 until PR101539 and by that changed > + the ABI on several targets. This flag is provided so that the backends > + can decide on the ABI with zero-width bitfields and emit -Wpsabi > + warnings. */ > +#define DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD(NODE) \ > + (DECL_BIT_FIELD (NODE) && (NODE)->decl_common.decl_flag_0) > +#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) > > /* Used in a FIELD_DECL to indicate that we cannot form the address of > this component. This makes it possible for Type-Based Alias Analysis > --- gcc/tree-streamer-out.c.jj 2021-06-02 10:08:14.381447116 +0200 > +++ gcc/tree-streamer-out.c 2021-09-02 10:34:58.377638627 +0200 > @@ -219,7 +219,10 @@ pack_ts_decl_common_value_fields (struct > bp_pack_value (bp, DECL_PACKED (expr), 1); > bp_pack_value (bp, DECL_NONADDRESSABLE_P (expr), 1); > bp_pack_value (bp, DECL_PADDING_P (expr), 1); > - bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1); > + if (DECL_BIT_FIELD (expr)) > + bp_pack_value (bp, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (expr), 1); > + else > + bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1); > bp_pack_value (bp, expr->decl_common.off_align, 8); > } > > --- gcc/tree-streamer-in.c.jj 2021-06-02 10:08:14.372447243 +0200 > +++ gcc/tree-streamer-in.c 2021-09-02 10:35:28.635204959 +0200 > @@ -256,7 +256,11 @@ unpack_ts_decl_common_value_fields (stru > DECL_PACKED (expr) = (unsigned) bp_unpack_value (bp, 1); > DECL_NONADDRESSABLE_P (expr) = (unsigned) bp_unpack_value (bp, 1); > DECL_PADDING_P (expr) = (unsigned) bp_unpack_value (bp, 1); > - DECL_FIELD_ABI_IGNORED (expr) = (unsigned) bp_unpack_value (bp, 1); > + unsigned val = (unsigned) bp_unpack_value (bp, 1); > + if (DECL_BIT_FIELD (expr)) > + SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (expr, val); > + else > + SET_DECL_FIELD_ABI_IGNORED (expr, val); > expr->decl_common.off_align = bp_unpack_value (bp, 8); > } > > --- gcc/lto-streamer-out.c.jj 2021-02-03 13:36:17.704980676 +0100 > +++ gcc/lto-streamer-out.c 2021-09-02 10:36:34.796256706 +0200 > @@ -1271,7 +1271,10 @@ hash_tree (struct streamer_tree_cache_d > hstate.add_flag (DECL_PACKED (t)); > hstate.add_flag (DECL_NONADDRESSABLE_P (t)); > hstate.add_flag (DECL_PADDING_P (t)); > - hstate.add_flag (DECL_FIELD_ABI_IGNORED (t)); > + if (DECL_BIT_FIELD (t)) > + hstate.add_flag (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (t)); > + else > + hstate.add_flag (DECL_FIELD_ABI_IGNORED (t)); > hstate.add_int (DECL_OFFSET_ALIGN (t)); > } > else if (code == VAR_DECL) > --- gcc/cp/class.c.jj 2021-09-01 21:30:30.473307484 +0200 > +++ gcc/cp/class.c 2021-09-02 10:36:16.860513770 +0200 > @@ -4634,7 +4634,7 @@ build_base_field (record_layout_info rli > DECL_FIELD_OFFSET (decl) = BINFO_OFFSET (binfo); > DECL_FIELD_BIT_OFFSET (decl) = bitsize_zero_node; > SET_DECL_OFFSET_ALIGN (decl, BITS_PER_UNIT); > - DECL_FIELD_ABI_IGNORED (decl) = 1; > + SET_DECL_FIELD_ABI_IGNORED (decl, 1); > } > > /* An empty virtual base causes a class to be non-empty > @@ -6658,7 +6658,7 @@ layout_class_type (tree t, tree *virtual > } > else if (might_overlap && is_empty_class (type)) > { > - DECL_FIELD_ABI_IGNORED (field) = 1; > + SET_DECL_FIELD_ABI_IGNORED (field, 1); > layout_empty_base_or_field (rli, field, empty_base_offsets); > } > else > @@ -6746,6 +6746,23 @@ layout_class_type (tree t, tree *virtual > normalize_rli (rli); > } > > + /* We used to remove zero width bitfields at this point since PR42217, > + while the C FE never did that. That caused ABI differences on various > + targets. Set the DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD 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)))) > + SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field, 1); > + > if (CLASSTYPE_NON_LAYOUT_POD_P (t) || CLASSTYPE_EMPTY_P (t)) > { > /* T needs a different layout as a base (eliding virtual bases > --- gcc/lto/lto-common.c.jj 2021-06-02 10:08:14.347447598 +0200 > +++ gcc/lto/lto-common.c 2021-09-02 10:35:49.480906189 +0200 > @@ -1187,6 +1187,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t > compare_values (DECL_NONADDRESSABLE_P); > compare_values (DECL_PADDING_P); > compare_values (DECL_FIELD_ABI_IGNORED); > + compare_values (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD); > compare_values (DECL_OFFSET_ALIGN); > } > else if (code == VAR_DECL) > > > Jakub >
--- 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