2012/12/13 Jakub Jelinek <ja...@redhat.com>:
> On Thu, Dec 13, 2012 at 11:29:47AM +0100, Richard Biener wrote:
>> On Thu, Dec 13, 2012 at 11:07 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>> > On Thu, Dec 13, 2012 at 11:03:58AM +0100, Richard Biener wrote:
>> >> struct X
>> >> {
>> >> char c;
>> >> short s;
>> >> char c2;
>> >> short s2;
>> >> } __attribute__((packed,aligned(2)));
>> >
>> > As struct-layout-1.exp tests show, this is something that was ABI-wise
>> > changed already several times. That said, for non-ms-bitfield-layout I'd
>> > strongly prefer if we could avoid yet another ABI change for it.
>>
>> Probably not exactly this case - 2.95, 3.2, 3.3, 4.1 ... all show the same
>> behavior. And Kai should have seen regressions, no?
>
> Maybe my memory is too weak, dunno if it was packed,aligned(N) or
> aligned(N),packed,
> but I remember seeing FAILs in such tests when testing against older
> compilers.
Well, I see the point about having packed and aligned in combination
only for final-element in struct/union. As packed enforces unaligned
accesses due it tries to save memory-use. If an user alignment is
present the packing can just happen on last member for struct, as
otherwise the alignment has to be applied and padding happens.
But I agree that I didn't intended to change sysv_abi here by this
patch. So I added to the change in start_record_layout a check to
ms-bitfields use.
2012-12-13 Kai Tietz
PR c/52991
* stor-layout.c (start_record_layout): Handle
packed-attribute for ms-structure-layout.
(update_alignment_for_field): Likewise.
(place_field): Likewise.
2012-12-12 Kai Tietz
PR c/52991
* gcc.dg/attr-ms_struct-packed2.c: New file.
* gcc.dg/attr-ms_struct-packed3.c: New file.
Ok for apply?
Regards,
Kai
Index: gcc/gcc/stor-layout.c
===================================================================
--- gcc.orig/gcc/stor-layout.c
+++ gcc/gcc/stor-layout.c
@@ -756,7 +756,10 @@ start_record_layout (tree t)
/* If the type has a minimum specified alignment (via an attribute
declaration, for example) use it -- otherwise, start with a
one-byte alignment. */
- rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t));
+ if (targetm.ms_bitfield_layout_p (t) && TYPE_PACKED (t))
+ rli->record_align = BITS_PER_UNIT;
+ else
+ rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t));
rli->unpacked_align = rli->record_align;
rli->offset_align = MAX (rli->record_align, BIGGEST_ALIGNMENT);
@@ -952,15 +955,20 @@ update_alignment_for_field (record_layou
meaningless. */
if (targetm.ms_bitfield_layout_p (rli->t))
{
+ if (rli->t && TYPE_PACKED (rli->t)
+ && (is_bitfield || !DECL_PACKED (field)
+ || DECL_SIZE (field) == NULL_TREE
+ || !integer_zerop (DECL_SIZE (field))))
+ desired_align = BITS_PER_UNIT;
/* Here, the alignment of the underlying type of a bitfield can
affect the alignment of a record; even a zero-sized field
can do this. The alignment should be to the alignment of
the type, except that for zero-size bitfields this only
applies if there was an immediately prior, nonzero-size
bitfield. (That's the way it is, experimentally.) */
- if ((!is_bitfield && !DECL_PACKED (field))
- || ((DECL_SIZE (field) == NULL_TREE
- || !integer_zerop (DECL_SIZE (field)))
+ else if ((!is_bitfield && !DECL_PACKED (field))
+ || ((DECL_SIZE (field) == NULL_TREE
+ || !integer_zerop (DECL_SIZE (field)))
? !DECL_PACKED (field)
: (rli->prev_field
&& DECL_BIT_FIELD_TYPE (rli->prev_field)
@@ -1414,7 +1422,12 @@ place_field (record_layout_info rli, tre
}
/* Now align (conventionally) for the new type. */
- type_align = TYPE_ALIGN (TREE_TYPE (field));
+ if (!TYPE_PACKED (rli->t))
+ {
+ type_align = TYPE_ALIGN (TREE_TYPE (field));
+ if (DECL_PACKED (field))
+ type_align = MIN (type_align, BITS_PER_UNIT);
+ }
if (maximum_field_alignment != 0)
type_align = MIN (type_align, maximum_field_alignment);
Index: gcc/gcc/testsuite/gcc.dg/attr-ms_struct-packed2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/attr-ms_struct-packed2.c
@@ -0,0 +1,31 @@
+/* Test for MS structure with packed attribute. */
+/* { dg-do run { target *-*-interix* *-*-mingw* *-*-cygwin*
i?86-*-darwin* i?86-*-linux* x86_64-*-linux* } }
+/* { dg-options "-std=gnu99" } */
+
+extern void abort ();
+
+struct A {
+ short s;
+ struct { int i; } __attribute__((__ms_struct__));
+} __attribute__((__ms_struct__, __packed__));
+
+struct B {
+ short s;
+ struct { int i; } __attribute__((__ms_struct__, __packed__));
+} __attribute__((__ms_struct__, __packed__));
+
+struct C {
+ struct { int i; } __attribute__((__ms_struct__));
+ short s;
+} __attribute__((__ms_struct__, __packed__));
+
+int
+main (void)
+{
+ if (sizeof (struct C) != sizeof (struct B)
+ || sizeof (struct A) != sizeof (struct B)
+ || sizeof (struct B) != 6)
+ abort ();
+
+ return 0;
+}
