[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #50 from matz at gcc dot gnu dot org 2006-02-14 16:13 --- Subject: Bug 22275 Author: matz Date: Tue Feb 14 16:12:56 2006 New Revision: 110982 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=110982 Log: PR middle-end/22275 * stor-layout.c (layout_decl): Zero-width bitfields aren't influenced by maximum_field_alignment or DECL_PACKED. (update_alignment_for_field): Ditto. (place_field): Ditto. * doc/extend.texi (#pragma pack, Type Attributes): Document this behaviour. Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/doc/extend.texi branches/gcc-4_1-branch/gcc/stor-layout.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #48 from mmitchel at gcc dot gnu dot org 2006-02-13 22:42 --- I'm going to comment on the behavior issue raised in the comments from David Moore, and then review the patch itself. First, the C standard has nothing to say about this issue; even were it not for the fact that the change only occurs within the scope of #pragma pack, both behaviors are conformant. Second, the ABI standards have nothing to say about this issue, because (so far as I am aware) none of them discuss #pragma pack. If the current behavior (i.e., GCC 4.1 as of today) had been the behavior of GCC for all time, I would not be in favor of making a change -- even though I think the current behavior is peculiar. However, the current behavior is itself a change; we need to resolve the situation once and for all, and we might as well do what seems best. As Michael and I have agreed, it seems to make sense that the zero-width bitfield directives have meaning based on their types; that makes the GNU-extended language (including #pragma pack) more expressive. I understand the user perception/marketing issue around ABI stability, and that's very important to me as well. However, this is very much a corner case (given the fact that #pragma pack is involved), and I think there are sound technical arguments for the change. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #49 from mmitchel at gcc dot gnu dot org 2006-02-13 22:59 --- I've approved this for 4.1 and mainline. http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01031.html Please do not apply to 4.0 or 3.4, as an ABI change along a release branch is even worse than an ordinary ABI change. Therefore, once applied to 4.1 and mainline, this PR can be closed. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #47 from matz at suse dot de 2006-02-12 03:59 --- What do you mean with 6 (as making more sense)? The size of the struct? Anyway, even ignoring that we talk about structs which are packed in various ways (as you rightly noticed) even the old (IMHO more sensible behaviour) fullfills the C standard you quoted. By aligning it to type it automatically makes a following bitfield not come to lie in the same unit (a byte usually), though that's obviously not the most strict interpretation of this rule. So it's not that the old behabiour would violate C99. What strengenths the case to go back actually are programs relieing on that behaviour, _and_ that it's more expressive. With the new behaviour there's no difference between char :0; short :0; int :0; If the user really only want to close the current unit he can write 'char :0'. But if he wants more alignment in a otherwise packed struct he has to play games currently, whereas with the pre-3.4 sematic he could have written 'int:0' (if int was his desired alignment for the next field). So, I still stand by my opinion that we should want to go back. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #45 from david dot moore at intel dot com 2006-02-10 22:34 --- The (C99) standard says: 6.7.2.1 (10) An implementation may allocate any addressable storage unit large enough to hold a bitfield. and 6.7.2.1 (11) As a special case, a bit-field structure member with a width of 0 indicates that no further bit-field is to be packed into the unit in which the previous bit-field, if any, was placed. NB -Not into a unit required by the given type (on the type :0 line) So looks like 6 is the more plausible answer and the old behavior seems to be bizarre at best. It means, in effect, that the compiler has to go back and change its mind about the size of unit in which the previous bitfield was to be allocated. (You can no doubt get around the use of the past tense in the standard by suitable invocation of an oracle, or two-pass compilation - you could also always allocate eveything in UINTS - yetch) A more compelling argument for not changing this back is that gcc has done a good job of being (C++) ABI compatible since 3.4. Changing this back to pre-3.4 behavior now will create an ABI break between 3.4 and 4.0.3. Even delaying the change till 4.1 would be bad enough. So I strongly represent that this change should not be made or, if it is made, it should only happen under control of a flag that is off by default. BTW, you may think I am arguing this because it would avoid having to change the Intel compiler but actually it looks like when we changed our behavior we did not make sure that our gcc 3.2 compatible behavior did not change, so it looks like we have work to do anyway, and the impact of this change on our work is perhaps an extra leg in a conditional. So I can claim altruism here. Actually the real reason is I have been telling people that the ABI has been very stable since 3.4 and I would rather not have egg on my face. -- david dot moore at intel dot com changed: What|Removed |Added CC||david dot moore at intel dot ||com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #46 from david dot moore at intel dot com 2006-02-11 00:14 --- (Note - I had not realized the importance of pragma pack to this problem. The fact that without it the behavior has not changed weakens my case, although it probably weakens the case that it should be restored to gcc 3.2 behavior by an equal amount) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change
--- Comment #44 from steven at gcc dot gnu dot org 2006-02-09 23:02 --- Ping! :-) Micha, are you going to take this bug? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #43 from rguenth at gcc dot gnu dot org 2006-01-27 23:25 --- Can we have an update on this or post the patch to get review please? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #39 from matz at suse dot de 2006-01-23 10:32 --- Gnah! It's even worse. I spoke too soon, and actually pre-3.4 (3.3 in fact) has the behaviour _swapped_. I.e. using the example struct I have these sizes (on i686): 3.3:normal 16, pragma 16, packed 12 4.1+patch: normal 12, pragma 12, packed 16 The 3.3 case is nice in the sense that the packed struct actually is smaller than the unpacked struct, but in a way it also means that 3.3 did somehow adjust the field offsets for the packed struct. What do we do? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #40 from matz at suse dot de 2006-01-23 11:21 --- Mark, re your comment #38: (my comment #39 actually came before, but I forgot to press Commit :-/ ) the #pragma pack(1) does not influence DECL_PACKED. It is only set by attribute(packed). That's why the difference of behaviour between a struct under #pragma pack(1) vs. a struct with an attribute packed occurs. I agree that it conceptually makes sense to implicitely have a zero-width bit-field never be DECL_PACKED (though this would deviate from pre-3.4). The ugly thing is, that current code in GCC seems to handle exactly this case differently. For instance in place_field(), we have this code: if (PCC_BITFIELD_TYPE_MATTERS ! targetm.ms_bitfield_layout_p (rli-t) TREE_CODE (field) == FIELD_DECL type != error_mark_node DECL_BIT_FIELD (field) ! DECL_PACKED (field) maximum_field_alignment == 0 ! integer_zerop (DECL_SIZE (field)) the body contains a call to ADJUST_FIELD_ALIGN then. So if this is a zero-sized bitfield, then this ADJUST won't be done no matter what DECL_PACKED is, and it seems that this is wanted here (in difference to the other place in layout_decl, where zero-sized bitfield simply weren't handled). The comment above this code says that it's purpose is compatibility with PCC, so perhaps struct with zero-sized bitfields weren't handled at all by PCC, and this is a non-issue. I don't know. Otherwise it might be, that both places need to be handled the same way to not risk inconsistencies. It currently looks as if it also isn't handled consistently right now (another call to ADJUST_... misses to test DECL_PACKED at all for instance). Double-sigh. Anyway, I'll attach my current patch which implements the suggested behaviour, including zero-bitfield == !DECL_PACKED in layout_decl. And also a small testprogram showing information about different struct under different settings. It shows inconsistencies in 3.3, and with the patch 4.1 is more consistent (plus the case in wine, namely of using #pragma pack(1) still does the same as pre-3.4). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #41 from matz at suse dot de 2006-01-23 11:23 --- Created an attachment (id=10710) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10710action=view) candidate patch This patch contains some commented out test code I had in for playing around. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #42 from matz at suse dot de 2006-01-23 11:28 --- Created an attachment (id=10711) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10711action=view) Testprogram This program generates the following output for 3.3-hammer-branch on x86-64: S_normal_i size 8 align 4 ofs 4 S_pragma_i size 8 align 1 ofs 4 S_packed_i size 12 align 1 ofs 8 S_pragma_packed_i size 12 align 1 ofs 8 S_normal_ll size 16 align 8 ofs 8 S_pragma_ll size 16 align 1 ofs 8 S_packed_ll size 16 align 1 ofs 8 S_pragma_packed_ll size 16 align 1 ofs 8 With -m32 it's: S_normal_i size 8 align 4 ofs 4 S_pragma_i size 8 align 1 ofs 4 S_packed_i size 8 align 1 ofs 4 S_pragma_packed_i size 8 align 1 ofs 4 S_normal_ll size 16 align 4 ofs 8 S_pragma_ll size 16 align 1 ofs 8 S_packed_ll size 12 align 1 ofs 4 S_pragma_packed_ll size 12 align 1 ofs 4 Note how 3.3 handled packed structs (in difference to those under #pragma) really strange. With 4.1 plus patch on x86-64: S_normal_i size 8 align 4 ofs 4 S_pragma_i size 8 align 1 ofs 4 S_packed_i size 8 align 1 ofs 4 S_pragma_packed_i size 8 align 1 ofs 4 S_normal_ll size 16 align 8 ofs 8 S_pragma_ll size 16 align 1 ofs 8 S_packed_ll size 16 align 1 ofs 8 S_pragma_packed_ll size 16 align 1 ofs 8 With -m32: S_normal_i size 8 align 4 ofs 4 S_pragma_i size 8 align 1 ofs 4 S_packed_i size 8 align 1 ofs 4 S_pragma_packed_i size 8 align 1 ofs 4 S_normal_ll size 12 align 4 ofs 4 S_pragma_ll size 12 align 1 ofs 4 S_packed_ll size 12 align 1 ofs 4 S_pragma_packed_ll size 12 align 1 ofs 4 So, it's at least consistent, and maybe even senseful -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #36 from matz at suse dot de 2006-01-20 14:01 --- Yes. Should be done shortly. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #37 from matz at suse dot de 2006-01-20 16:36 ---
Hmpf. One more difficulty. x86 uses the ADJUST_FIELD_ALIGN macro
to further fiddle with alignments of fields. On x86 this is used to
adjust the alignment of long long to 4 (instead of the natural 8).
This is used only when the field is not DECL_PACKED (makes sense).
This has the funny side-effect that a struct containing a long long zero-width
bitfield aligns to 4 for unpacked and to 8 for packed structs, i.e. the
packed struct actually is _larger_ than the unpacked struct. E.g. the
running example with UINT being long long:
typedef int BOOL;
typedef unsigned long long UINT;
#pragma pack(1)
typedef struct {
BOOL fFullPathTitle:1;
BOOL fSaveLocalView:1;
BOOL fNotShell:1;
BOOL fSimpleDefault:1;
BOOL fDontShowDescBar:1;
BOOL fNewWindowMode:1;
BOOL fShowCompColor:1;
BOOL fDontPrettyNames:1;
BOOL fAdminsCreateCommonGroups:1;
UINT fUnusedFlags:7;
UINT :0;
UINT fMenuEnumFilter;
} CABINETSTATE;
This struct being unpacked (only influenced by #pragma) has size 12 after
my patch on i686, and size 16 (!) when being packed via attribute.
What's even more ugly is that pre-3.4 GCC had a size of 16 for both
cases (pragma or attribute packed) on i686 :-(
So, how would we like to handle this? Doing as pre 3.4 did is probably
possible but not trivially done. Basically the code doing this is:
if (! DECL_USER_ALIGN (decl) ! DECL_PACKED (decl))
{
#ifdef ADJUST_FIELD_ALIGN
DECL_ALIGN (decl) = ADJUST_FIELD_ALIGN (decl, DECL_ALIGN (decl));
#endif
}
Now, I could just ignore DECL_PACKED for zero-width bitfields, then the
adjustment would be done for both cases and we had a size of 12 with
attribute or pragma, i.e. not the same as pre 3.4 in both. I also
could never adjust zero-width bitfields, so that they would get their
natural alignment even when the target wanted something else. Then both
cases would have size 16, being the same as pre 3.4.
I'm leaning towards not doing field adjustments for zero-width bitfields
at all, having the effect that a zero-width bitfield has a user-alignment
set explicitely (of it's base type). I think if one understands zero-width
bitfields purely as alignment constraints than this implicit DECL_USER_ALIGN
behaviour seems sensible.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #38 from mark at codesourcery dot com 2006-01-20 18:02 ---
Subject: Re: [3.4/4.0/4.1/4.2 Regression] bitfield
layout change (regression?)
matz at suse dot de wrote:
--- Comment #37 from matz at suse dot de 2006-01-20 16:36 ---
Hmpf. One more difficulty. x86 uses the ADJUST_FIELD_ALIGN macro
to further fiddle with alignments of fields. On x86 this is used to
adjust the alignment of long long to 4 (instead of the natural 8).
This is used only when the field is not DECL_PACKED (makes sense).
This has the funny side-effect that a struct containing a long long zero-width
bitfield aligns to 4 for unpacked and to 8 for packed structs
Ugh. I think we can all agree that these issues had not been well
thought through previously. :-)
if (! DECL_USER_ALIGN (decl) ! DECL_PACKED (decl))
{
#ifdef ADJUST_FIELD_ALIGN
DECL_ALIGN (decl) = ADJUST_FIELD_ALIGN (decl, DECL_ALIGN (decl));
#endif
}
Now, I could just ignore DECL_PACKED for zero-width bitfields, then the
adjustment would be done for both cases and we had a size of 12 with
attribute or pragma, i.e. not the same as pre 3.4 in both.
I don't think a zero-width bitfield should ever be DECL_PACKED. (In
this case, it's DECL_PACKED because the structure is in the scope of
#pragma pack(1).)
In other words, I think a zero-width bitfield should always be subject
to ADJUST_FIELD_ALIGN; the meaning of a zero-width bitfield of type T
(for PCC_BITFIELD_TYPE_MATTERS) should be align the next field as you
would a field of type T.
I'm leaning towards not doing field adjustments for zero-width bitfields
at all, having the effect that a zero-width bitfield has a user-alignment
set explicitely (of it's base type).
Careful! That would be an ABI change even in the non-packed case, which
we don't want to do.
I think the best change would be just not to mark zero-width bitfields
as DECL_PACKED in the first place; a second choice, if easier, would be
to disregard the DECL_PACKED setting in stor-layout.c. Please add a
test case for this new oddity you discovered, if you would.
Thanks,
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #32 from matz at suse dot de 2006-01-19 14:44 --- Mark, I agree that it's saner when both structures (with #pragma pack and attribute packed) have the same length of 8 on i686 and x86_64 (because the bitfield was declared 'int' in difference to 'long' for instance). Then I have a question to clarify if I understood correctly: by remembering the original maximum_field_alignment and using that for zero-sized bitfields you want to use the absolute first, default, m_f_a, or the one last set before the innermost #pragma pack? Consider an example like so, and lets assume the initial max field alignment was 4: mfa == 4 #pragma pack(2) // 1 mfa == 2 #pragma pack(1) // 2 mfa == 1 #pragma pack() // 3 mfa == 4 #pragma pack (push,2) // 4 mfa == 2 #pragma pack (push,1) // 5 mfa == 1 #pragma pop // 6 mfa == 2 #pragma pop // 7 mfa == 4 With what would you constrain the alignment of a zero sized bitfield at each of the seven points? What if the initial mfa is 0 (i.e. not set)? Should -fpack-struct=... (which influences the initial mfa) influence that constraint too, or not? My opinion is, that at each of the seven points above we should constrain with the initial mfa (i.e. 4 in the example above), as adjusted by the -fpack-struct command line option. That would have the effect of aligning zero sized bitfield at max to the architecture default (possibly adjusted globally by the cmdline option), while effectively ignoring all #pragma packs in effect. I think that is what we want the semantics of a zero-sized bitfield to be. Agreed? Another point: If we make the structure with attribute packed on both x86 and x86-64 be eight long (to agree with the behaviour of using pragma), then we do add another variant unfortunately. In pre 3.4 that structure was 12 on x86-64 (which I think was an actual error). Wine itself uses only #pragma pack AFAIK, so it wouldn't be affected by this change. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #33 from mark at codesourcery dot com 2006-01-19 16:59 --- Subject: Re: [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?) matz at suse dot de wrote: --- Comment #32 from matz at suse dot de 2006-01-19 14:44 --- Mark, I agree that it's saner when both structures (with #pragma pack and attribute packed) have the same length of 8 on i686 and x86_64 (because the bitfield was declared 'int' in difference to 'long' for instance). Then I have a question to clarify if I understood correctly: by remembering the original maximum_field_alignment and using that for zero-sized bitfields you want to use the absolute first, default, m_f_a, or the one last set before the innermost #pragma pack? Consider an example like so, and lets assume the initial max field alignment was 4: mfa == 4 #pragma pack(2) // 1 mfa == 2 #pragma pack(1) // 2 mfa == 1 #pragma pack() // 3 mfa == 4 #pragma pack (push,2) // 4 mfa == 2 #pragma pack (push,1) // 5 mfa == 1 #pragma pop // 6 mfa == 2 #pragma pop // 7 mfa == 4 With what would you constrain the alignment of a zero sized bitfield at each of the seven points? I'm suggesting that a zero-width bitfield have the same alignment influence at all 7 points. In your example, mfa would be effectively four at all 7 points. What if the initial mfa is 0 (i.e. not set)? Then, it should be zero at all seven points, meaning that a zero-width bitfield of type T would force alignment to whatever alignment a field of type T would have. Should -fpack-struct=... (which influences the initial mfa) influence that constraint too, or not? That's harder, but I think it should. The point is basically that, independent of #pragma pack, a zero-width bitfield should give you the alignment it would give you oustide of the #pragma packs. My opinion is, that at each of the seven points above we should constrain with the initial mfa (i.e. 4 in the example above), as adjusted by the -fpack-struct command line option. Good, we agree! Another point: If we make the structure with attribute packed on both x86 and x86-64 be eight long (to agree with the behaviour of using pragma), then we do add another variant unfortunately. In pre 3.4 that structure was 12 on x86-64 (which I think was an actual error). That's unfortunate, but given that things are moving around, I think we should go for maximum sanity. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #34 from steven at gcc dot gnu dot org 2006-01-19 17:40 --- I looked up a few links to see how people use zero-length bit-fields and what semantics they're expecting. I mostly found links to compiler documentation about how other compilers interpret these bit-fields. Perhaps we can verify that GCC 4.1 with a patch for this bug satisfies those expectations. If they don't, perhaps those deviations should be mentioned explicitly in the release notes and the manual. Intel ICC behavior was already noted in comment #14. IBM (XLC): http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.vacpp7a.doc/proguide/ref/calgnbit.htm A zero-length bit field pads to the next alignment boundary of its base declared type. This causes the next member to begin on a 4-byte boundary for all types except long in 64-bit mode and long long in both 32-bit and 64-bit mode, which will move the next member to the next 8-byte boundary. Padding does not occur if the previous member's memory layout ended on the appropriate boundary. And the rules from Rules for bit-packed alignment in the XL C/C++ Advanced Edition for LinuxProgramming Guide Version 7.0 state for bit-packed structs: If a zero-length bit field is the first member of an aggregate, it has no effect on boundary. If the zero-length bit field is already at a byte boundary, the next member starts at this boundary. A non-bit field member that follows a bit field is aligned on the next bit boundary. I don't know if bit-packed is the same as just packed in GCC. Microsoft: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccelng/htm/class_34.asp Has an example saying unsigned 0;// Force alignment to next boundary., which is not very specific (but better than nothing I guess). HP, HP C manual for VAX and other older HP systems IIUC (http://docs.hp.com/en/B3901-90012/ch02s01.html): zero-length bit-fields force the following member to the next natural boundary. And from HP's Itanium Software Conventions and Runtime Architecture (http://www.desktalk.com/drc/resources/ia64rt-12-gen.pdf): Zero-length bit fields force the alignment of the following member of a structure to the next alignment boundary corresponding to the type of the bit field. An unnamed zero-length bit field, however, will not force the external alignment of the structure to that boundary. And last but not least, HP's OpenVMS C compiler (http://h71000.www7.hp.com/commercial/c/docs/6180p012.html#index_x_361): If the bit field is assigned a width of 0, it indicates that no further bit fields should be placed in the alignment unit[.] (...) As a special case, an unnamed [bit-]field of width 0 causes the next member (normally another field) to be aligned on at least a byte boundary; that is, a bit-field structure member with zero width indicates that no further bit field should be packed into an alignment unit. (...) [T]he second is zero bits wide, which forces the next member to be aligned on a natural or byte boundary. So in summary: - Intel 9.0 is just as buggy as GCC 3.4 and GCC 4.0 because it apparently tries to even mimick GCC bugs now. I think ICC/GCC compatibility is relevant but we can't stop fixing our bugs because Intel adopts them too. (Besides, we don't know what ICC does on x86-64 and for e.g. long long). - Older HP compilers and MS compilers use zero-length bit-fields to force the following member of a struct to the next natural (is that GCC's BITS_PER_WORD?) boundary. - XLC and HP on IA-64 force alignment corresponding to the type of the bit field, of which IBM says it is essentially the same, but this is not true for GCC with e.g. long long on ix86. I also found this post from Mark M. about PCC_BITFIELD_TYPE_MATTERS which may be of interest: http://gcc.gnu.org/ml/gcc/2002-12/msg01324.html. This is the first time I've seen the purpose of PCC_BITFIELD_TYPE_MATTERS in a way that I can understand ;-) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #35 from mark at codesourcery dot com 2006-01-19 19:14 --- Subject: Re: [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?) steven at gcc dot gnu dot org wrote: - Older HP compilers and MS compilers use zero-length bit-fields to force the following member of a struct to the next natural (is that GCC's BITS_PER_WORD?) boundary. - XLC and HP on IA-64 force alignment corresponding to the type of the bit field, of which IBM says it is essentially the same, but this is not true for GCC with e.g. long long on ix86. That's essentially the PCC_BITFIELD_TYPE_MATTERS distinction; the HP/MS compilers in the first bullet point are not using the pcc-style bitfields. Your research is interesting. However, I'm not too worried about our handling of zero-width bitfields by themselves; the various psABIs specify that behavior, so we have a clear guideline. It's when we throw in various GCC extensions (#pragma pack, __attribute__((aligned)), etc.) that it gets complicated and hard. I'm hoping that since Michael and I independently came to the same conclusion about how things should work, and since there were no objections to my initial proposal to change things back on the mailing list, we're actually pretty close to a patch. Michael, do you plan to revise your patch to implement the semantics on which we've agreed? Thanks, -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #29 from mark at codesourcery dot com 2006-01-18 23:00 --- Subject: Re: [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?) I think that we should do as follows. Preserve the original value of maximum_field_alignment when doing #pragma pack. Then, for zero-width bitfields, we should align to the minimum of the original maximum_field_alignment and the otherwise natural alignment. The difference between this and the last proposed patch is that I don't think we should entirely ignore maximum_field_alignment for zero-width bitfields; if long long as a field will only have (say) 2-byte alignment on some embedded target where structure-packing is the default, then a long long : 0; bitfield should only force 4-byte alignment. However, that's an abstract argument; I'm not actually sure what existing practice was with older versions of GCC. Again, in the abstract, I think the example in Comment #12 ought to have size 8 on both IA32 and AMD64 architectures. I can't see any good justification for size 12, with a PCC_BITFIELD_TYPES_MATTER ABI. And, I think that the size of the structure with #pragma pack(1) ought to be the same as with __attribute__((packed)). So, my concern with the patch in Comment #12 is that it would ignore the pre-set maximum_field_alignment on targets with default structure packing; other than that, I think it looks fine. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #30 from steven at gcc dot gnu dot org 2006-01-18 23:08 --- We should at least avoid introducing a third variant of how we lay out these nasty zero-sized friends. People actually use them. For example Wine uses them to enforce compatible alignment and data layout with MS Windows datastructure. Wine has a bunch of #ifdefs spread around in its sources to make it work with the pre- and post-GCC 3.4 layout. Adding a third would drive those poor people nuts. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #31 from mark at codesourcery dot com 2006-01-18 23:28 --- Subject: Re: [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?) steven at gcc dot gnu dot org wrote: --- Comment #30 from steven at gcc dot gnu dot org 2006-01-18 23:08 --- We should at least avoid introducing a third variant of how we lay out these nasty zero-sized friends. People actually use them. For example Wine uses them to enforce compatible alignment and data layout with MS Windows datastructure. Wine has a bunch of #ifdefs spread around in its sources to make it work with the pre- and post-GCC 3.4 layout. Adding a third would drive those poor people nuts. I agree -- but I don't think we're talking about a third variant. Michael's patch looks to me like it will restore the pre-3.4 behavior, which everyone agrees makes more sense. My issue with respect to maximum_field_alignment doesn't really apply to pre-4.0 toolchains, since they didn't have default structure packing for targets, AFAICT. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #26 from hubicka at gcc dot gnu dot org 2006-01-17 21:07
---
Hi,
I've looked into it for some time, so here is my POV of this ugly issue.
It seems to me that from documentation of EMPTY_FIELD_BOUNDARY in gccint it is
clear that it
should be ignored when PPC_BITFIELD_TYPE_MATTERS is set. This seems pretty
correct in Jason's version of code to me.
Corcerning the value of EMPTY_FIELD_BOUNDARY in i386.h, I actually introduced
it back in 2001 while doing the 64bit changes. I am pretty sure I didn't much
worried about actual meaning of the value,
I just looked for 32 and was trying to decide whether the value means 32 or
size of word. The alignment seemed to be more word related.
The behaviour in 3.0.4 is to set the alignment according to type of bitfield
(is PPC_BITFIELD_TYPE_MATTERS). So struct a {char a; short :0;} has size 2.
This seems to be handled by the last hunk overwriting EMPTY_FIELD_BOUDNARY
Steven quotes in his analysis that has no equivalent in Jason's code.
I think it should be added into the conditional PPC_BITFIELD_TYPE_MATTERS in
stor layout at the place PPC_BITFIELD_TYPE_MATTERS is processed now, so I will
try to test the patch tomorrow.
I would also suggest removing the ignored EMPTY_FIELD_BOUNDARY from i386.h.
Does something from the above seem to make sense? (this is really slipperly)
Honza
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #27 from matz at suse dot de 2006-01-17 22:12 ---
Funnily I've also looked at stor-layout.c a bit, and basically came to
a similar conclusion and patch like Steven. I agree that as per documentation
PCC_BITFIELD_TYPE_MATTERS overrides EMPTY_FIELD_BOUNDARY. But that was
also a change by Jasons patch. Formerly it just influenced how empty
fields are handled. Clearly it influenced it in a different way than simple
overriding.
The problem, like Steven already analyzed, is twofold:
1) maximum_field_alignment now affects also empty bitfields,
which it didn't before, because before Jason maximum_field_alignment
was evaluated before other things were taken into account, and now
it's the last thing done. That's why earlier something larger than
alignment 8 bit was possible with #pragma pack(1) at all.
2) A bug or feature in pre-3.4 lead to the ignoring of EMPTY_FIELD_ALIGNMENT
when larger than 32 in our case. Namely because the initial alignment
of 64 (as required by EMPTY_FIELD_ALIGNMENT) was overriden by the
alignment of the type of the bitfield (int here, i.e. 32 bit).
I've come up with this simple patch for the problem, which fixes the testcase
for i386 and x86-64 (in the sense of being compatible with = 3.3) .
The idea is to simply ignore the max field alignment
for empty bitfield (hence falling back to either PCC_BITFIELD_TYPE_MATTERS
or EMPTY_FIELD_ALIGNMENT as the target requested).
This needs to be tested also with struct where the packed property is not
due to a #pragma pack(1) but rather a packed attribute, or similar.
Index: stor-layout.c
===
--- stor-layout.c (revision 107699)
+++ stor-layout.c (working copy)
@@ -337,6 +337,7 @@
/* For fields, it's a bit more complicated... */
{
bool old_user_align = DECL_USER_ALIGN (decl);
+ bool zero_bitfield = false;
if (DECL_BIT_FIELD (decl))
{
@@ -348,6 +349,7 @@
! DECL_PACKED (decl)
! targetm.ms_bitfield_layout_p (DECL_FIELD_CONTEXT (decl)))
{
+ zero_bitfield = true;
#ifdef PCC_BITFIELD_TYPE_MATTERS
if (PCC_BITFIELD_TYPE_MATTERS)
do_type_align (type, decl);
@@ -428,7 +430,7 @@
}
/* Should this be controlled by DECL_USER_ALIGN, too? */
- if (maximum_field_alignment != 0)
+ if (maximum_field_alignment != 0 ! zero_bitfield)
DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), maximum_field_alignment);
}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #28 from matz at suse dot de 2006-01-17 22:31 ---
And indeed with this testcase:
typedef int BOOL;
typedef unsigned int UINT;
typedef struct {
BOOL fFullPathTitle:1;
BOOL fSaveLocalView:1;
BOOL fNotShell:1;
BOOL fSimpleDefault:1;
BOOL fDontShowDescBar:1;
BOOL fNewWindowMode:1;
BOOL fShowCompColor:1;
BOOL fDontPrettyNames:1;
BOOL fAdminsCreateCommonGroups:1;
UINT fUnusedFlags:7;
UINT :0;
UINT fMenuEnumFilter;
} __attribute__((packed)) CABINETSTATE;
int f[sizeof(CABINETSTATE) == 8? 1 : -1];
---
it's still broken with the patch. gcc 3.3 has size 8 on i686 and size 12
on x86-64. With some other fix to my patch (below) I get 8 and 8 (without
that fix it's 6 and 6). This
is consistent with the old #pragma pack(1) behaviour, so arguably this was
an inconsistency in 3.3, worth to be fixed, but it's still a change
in behaviour. It would be interesting to know what earlier compilers had.
The mentioned fix is ignoring zero sized bitfields also when DECL_PACKED
is set, like so:
Index: stor-layout.c
===
--- stor-layout.c (revision 107699)
+++ stor-layout.c (working copy)
@@ -337,6 +337,7 @@
/* For fields, it's a bit more complicated... */
{
bool old_user_align = DECL_USER_ALIGN (decl);
+ bool zero_bitfield = false;
if (DECL_BIT_FIELD (decl))
{
@@ -345,9 +346,9 @@
/* A zero-length bit-field affects the alignment of the next
field. */
if (integer_zerop (DECL_SIZE (decl))
- ! DECL_PACKED (decl)
! targetm.ms_bitfield_layout_p (DECL_FIELD_CONTEXT (decl)))
{
+ zero_bitfield = true;
#ifdef PCC_BITFIELD_TYPE_MATTERS
if (PCC_BITFIELD_TYPE_MATTERS)
do_type_align (type, decl);
@@ -408,6 +409,7 @@
check old_user_align instead. */
if (DECL_PACKED (decl)
!old_user_align
+ !zero_bitfield
(DECL_NONADDRESSABLE_P (decl)
|| DECL_SIZE_UNIT (decl) == 0
|| TREE_CODE (DECL_SIZE_UNIT (decl)) == INTEGER_CST))
@@ -428,7 +430,7 @@
}
/* Should this be controlled by DECL_USER_ALIGN, too? */
- if (maximum_field_alignment != 0)
+ if (maximum_field_alignment != 0 ! zero_bitfield)
DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), maximum_field_alignment);
}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #22 from rguenth at gcc dot gnu dot org 2006-01-16 10:22 --- Honza or Micha - can you take over from here? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #23 from matz at suse dot de 2006-01-16 15:14 --- The x86-64 ABI itself doesn't talk about zero-sized bitfields. So both behaviours are correct regarding the ABI. It talks about unnamed bitfields (which zero-sized ones must be) not influencing the overall alignment of structures or unions, but the problem here is different. Having said that I agree with Marks mail on gcc@ that the pre-3.4 behaviour made more sense. Unfortunately I'm also no stor-layout.c expert, so can't really comment on how the best approach is to implement it. I assume Jason would be the best to comment here, as he changed that behaviour. Stevens latest patch changes the evaluation of EMPTY_FIELD_BOUNDARY vs. PCC_BITFIELD_TYPE_MATTERS, so someone needs to make sure that this is okay. Additionally I don't know how stor-layout tracks alignment, i.e. if desired_align contains the alignment for the _current_ field, or for the _next_ field. A zero-sized bitfield should influence alignment of the next field (although due to the size of zero this shouldn't make a difference). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #24 from steven at gcc dot gnu dot org 2006-01-16 23:31 --- Realistically, I don't think it's safe to still fix this for GCC 4.1. The whole thing looks so complicated (to me at least) that the risk of introducing new bugs just before the release is significant. It is unfortunate that Jason has completely ignored this regression, even though he caused it. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #25 from steven at gcc dot gnu dot org 2006-01-16 23:33 --- Re. comment #23, iiuc the old behavior was to align the zero-length bitfield, not the next field. My patch (one of them anyway) switches the order of EMPTY_FIELD_BOUNDARY and PCC_BITFIELD_TYPE_MATTERS back to how it was before Jason's change, but that change alone is not enough to fix the bug. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #16 from steven at gcc dot gnu dot org 2006-01-14 13:56 ---
With this patch we force the alignment, but I have to admit I have no idea if
this approach is correct. The only other place in stor-layout.c where we look
at EMPTY_FIELD_BOUNDARY is in layout_decl, but that code is never reached for
targets that define PCC_BITFIELD_TYPE_MATTERS.
Index: stor-layout.c
===
--- stor-layout.c (revision 109701)
+++ stor-layout.c (working copy)
@@ -1235,6 +1235,20 @@ place_field (record_layout_info rli, tre
rli-bitpos = bitsize_zero_node;
rli-offset_align = MIN (rli-offset_align, desired_align);
}
+#ifdef EMPTY_FIELD_BOUNDARY
+ else if (integer_zerop (DECL_SIZE (field))
+ ! (*targetm.ms_bitfield_layout_p) (DECL_FIELD_CONTEXT (field)))
+{
+ /* If FIELD is a zero-bit bitfield, force alignment on the next
+EMPTY_FIELD_BOUNDARY. */
+ tree x = size_binop (MINUS_EXPR,
+ build_int_cst (TREE_TYPE (DECL_SIZE (field)),
+ EMPTY_FIELD_BOUNDARY),
+ rli-bitpos);
+ rli-bitpos = size_binop (PLUS_EXPR, rli-bitpos, x);
+ normalize_rli (rli);
+}
+#endif
else
{
rli-bitpos = size_binop (PLUS_EXPR, rli-bitpos, DECL_SIZE (field));
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #17 from steven at gcc dot gnu dot org 2006-01-14 13:57 --- If the approach is good, we should at least be checking (DECL_BIT_FIELD (field)) also before forcing things... -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #18 from steven at gcc dot gnu dot org 2006-01-14 14:48 --- This code in layout_decl looks suspicous: /* Should this be controlled by DECL_USER_ALIGN, too? */ if (maximum_field_alignment != 0) DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), maximum_field_alignment); For this test case, maximum_field_alignment==8, so every bitfield is at most byte aligned by these two lines. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #19 from steven at gcc dot gnu dot org 2006-01-14 15:17 --- On AMD64, EMPTY_FIELD_BOUNDARY is 64 bits, so if we honnor that for the zero-length bitfield, we get a size of 12 for the struct in the test case in comment #9 (where we apparently expect 8, which is what you get with EMPTY_FIELD_BOUNDARY==32 even on AMD64). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #20 from steven at gcc dot gnu dot org 2006-01-14 15:35 --- Indeed, in GCC 3.2 (GNU C version 3.2.3 (x86_64-unknown-linux-gnu)) initially we have a DECL_ALIGN of 64 bits for the zero-length bitfield when we enter field_decl: Breakpoint 2, place_field (rli=0xa15b30, field=0x2ab34820) at stor-layout.c:727 727 tree type = TREE_TYPE (field); 1: debug_tree (field) = field_decl 0x2ab34820 type integer_type 0x2aae7750 unsigned int unsigned SI size integer_cst 0x2aae58d0 constant 32 unit size integer_cst 0x2aae5a20 constant 4 align 32 symtab 0 alias set -1 precision 32 min integer_cst 0x2aae5ab0 0 max integer_cst 0x2aae5ae0 4294967295 bit-field nonaddressable decl_4 VOID file t.c line 6 size integer_cst 0x2aaee870 type integer_type 0x2aaf09c0 bit_size_type constant 0 align 64 offset_align 1 context record_type 0x2ab349c0 chain field_decl 0x2ab348f0 fMenuEnumFilter void (gdb) undisp 1 (gdb) p field-decl.u1.a.align $1 = 64 (gdb) We go on to layout_decl then with the following parameters: 775 layout_decl (field, known_align); (gdb) p known_align $2 = 16 (gdb) p desired_align $3 = 64 (gdb) p user_align $4 = 0 (gdb) next 776 if (! DECL_PACKED (field)) (gdb) p field-decl.u1.a.align $5 = 8 So layout_decl has overruled the desired alignment of the field from 64 bits to 8 bits. Then we later on change the desired alignment again: 799 if ((* targetm.ms_bitfield_layout_p) (rli-t) (gdb) 817 if (PCC_BITFIELD_TYPE_MATTERS type != error_mark_node (gdb) 826 if (! integer_zerop (DECL_SIZE (field))) (gdb) 828 else if (! DECL_PACKED (field)) (gdb) 829 desired_align = TYPE_ALIGN (type); (gdb) 833 if (DECL_NAME (field) != 0) (gdb) p desired_align $8 = 32 Then we change the rli-bitpos to match the latest desire_align on lines 880-908 and we lay out the next field on the 32th bit in the record. In other words, it looks like effectively the pre-3.4 ABI for AMD64 accidentally updated the alignment to 32 bits (instead of 64 as requested). To return to the pre-3.4 ABI, we also need target changes to recognize zero-length bit fields and make EMPTY_FIELD_BOUNDARY return 32 bits for them. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #21 from steven at gcc dot gnu dot org 2006-01-14 15:55 --- Created an attachment (id=10641) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10641action=view) Fix x86_field_alignment for AMD64 zero-length bitfields; try to revert to pre-jason ABI So I'll just admit I have no idea how to fix this. I've attached my latest effort, but this layout issue is complicated enough that someone _very_ familiar with stor-layout should look at this bug if we're going to have a safe fix for GCC 4.1. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
[Bug middle-end/22275] [3.4/4.0/4.1/4.2 Regression] bitfield layout change (regression?)
--- Comment #15 from mmitchel at gcc dot gnu dot org 2005-12-20 02:34 --- In this message: http://gcc.gnu.org/ml/gcc/2005-12/msg00558.html I suggested that we switch GCC 4.1 back to the pre-3.4 behavior and asked if there were objections. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22275
