[Bug c/52238] New: -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[Bug c/52238] New: -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[michael.kostylev at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

             Bug #: 52238
           Summary: -mms-bitfields: __attribute__ ((aligned (n))) ignored
                    for struct members
    Classification: Unclassified
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: michael.kostylev@gmail.com


The following test case

% cat > test.c << __EOF && gcc -mms-bitfields test.c && { ./a.out ; echo $? ; }
struct {
    char a;
    char b __attribute__ ((aligned (16)));
} s;

int main()
{
    return (__PTRDIFF_TYPE__)&s.b & 15;
}
__EOF

returns 1, since s.b is not 16-byte aligned (with -mno-ms-bitfields it returns
0, as expected). The test passes with 4.1.2, but fails with 4.2.1 and above.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-14
     Ever Confirmed|0                           |1
      Known to fail|                            |4.7.0

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-14 12:22:01 UTC ---
Confirmed.  The alignment doesn't seem to be ignored though, but the fields
position is not updated appropriately.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktietz at gcc dot gnu.org

--- Comment #2 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-15 18:34:40 UTC ---
in stor-layout.c function place_field() it is said "We already align ms_struct
fields, so don't re-align them".  This isn't completely true, as desired_align
isn't handled in ms_struct case.

Following patch seems to solve this issue:

Index: stor-layout.c
===================================================================
--- stor-layout.c       (revision 184262)
+++ stor-layout.c       (working copy)
@@ -1141,15 +1141,14 @@
     }

   /* Does this field automatically have alignment it needs by virtue
-     of the fields that precede it and the record's own alignment?
-     We already align ms_struct fields, so don't re-align them.  */
-  if (known_align < desired_align
-      && !targetm.ms_bitfield_layout_p (rli->t))
+     of the fields that precede it and the record's own alignment?  */
+  if (known_align < desired_align)
     {
       /* No, we need to skip space before this field.
         Bump the cumulative size to multiple of field alignment.  */

-      if (DECL_SOURCE_LOCATION (field) != BUILTINS_LOCATION)
+      if (!targetm.ms_bitfield_layout_p (rli->t)
+          && DECL_SOURCE_LOCATION (field) != BUILTINS_LOCATION)
        warning (OPT_Wpadded, "padding struct to align %q+D", field);

       /* If the alignment is still within offset_align, just align
@@ -1302,7 +1301,7 @@
                 type size!) */
              HOST_WIDE_INT bitsize = tree_low_cst (DECL_SIZE (field), 1);

-             if (rli->remaining_in_alignment < bitsize)
+             if (rli->remaining_in_alignment < bitsize) /* $$$$ */
                {
                  HOST_WIDE_INT typesize = tree_low_cst (TYPE_SIZE (type), 1);

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[michael.kostylev at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #3 from Michael Kostylev <michael.kostylev at gmail dot com> 2012-02-16 13:26:24 UTC ---
Ok, let's modify the test case - s/char a;/char a:6;/:

struct {
    char a:6;
    char b __attribute__ ((aligned (16)));
} s;

int main()
{
    return (__PTRDIFF_TYPE__)&s.b & 15;
}

The b member is still misaligned, moreover, its offset is equal to 0x11 after
patching.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #4 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-16 14:03:29 UTC ---
Hmm, right.  The previous field needs to be cleared for ms-bitfields, too.

Index: stor-layout.c
===================================================================
--- stor-layout.c       (revision 184287)
+++ stor-layout.c       (working copy)
@@ -1141,15 +1141,14 @@
     }

   /* Does this field automatically have alignment it needs by virtue
-     of the fields that precede it and the record's own alignment?
-     We already align ms_struct fields, so don't re-align them.  */
-  if (known_align < desired_align
-      && !targetm.ms_bitfield_layout_p (rli->t))
+     of the fields that precede it and the record's own alignment?  */
+  if (known_align < desired_align)
     {
       /* No, we need to skip space before this field.
         Bump the cumulative size to multiple of field alignment.  */

-      if (DECL_SOURCE_LOCATION (field) != BUILTINS_LOCATION)
+      if (!targetm.ms_bitfield_layout_p (rli->t)
+          && DECL_SOURCE_LOCATION (field) != BUILTINS_LOCATION)
        warning (OPT_Wpadded, "padding struct to align %q+D", field);

       /* If the alignment is still within offset_align, just align
@@ -1171,7 +1170,8 @@

       if (! TREE_CONSTANT (rli->offset))
        rli->offset_align = desired_align;
-
+      if (targetm.ms_bitfield_layout_p (rli->t))
+        rli->prev_field = NULL;
     }

   /* Handle compatibility with PCC.  Note that if the record has any

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[rafael.carre at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #5 from Rafaël Carré <rafael.carre at gmail dot com> 2012-02-17 00:12:49 UTC ---
Second patch tested with VLC.

After rebuilding gcc 4.6.2 package from Debian, mingw-w64-dev package with new
gcc, and VLC including all libraries (qt4, ffmpeg etc), I got DXVA2 decoding
running perfectly.

Previously it would give uncorrectly decoded pictures because of different
struct layout given to the GPU drivers.

So it works fine for me.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[michael.kostylev at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #6 from Michael Kostylev <michael.kostylev at gmail dot com> 2012-02-17 11:02:50 UTC ---
According to FATE (the Libav testsuite), the patch fixes the issue. Thanks.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[michael.kostylev at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #7 from Michael Kostylev <michael.kostylev at gmail dot com> 2012-02-18 10:04:41 UTC ---
It does not play a role here, nonetheless __SIZE_TYPE__ should be more correct
than __PTRDIFF_TYPE__.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #8 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-18 21:57:21 UTC ---
(In reply to comment #7)
> It does not play a role here, nonetheless __SIZE_TYPE__ should be more correct
> than __PTRDIFF_TYPE__.

I think here is the question for your reply missing in this bug-report.

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #9 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-20 22:05:12 UTC ---
Author: ktietz
Date: Mon Feb 20 22:05:08 2012
New Revision: 184409

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184409
Log:
    PR target/52238
    * stor-layout.c (place_field): Handle desired_align for
    ms-bitfields, too.

    * gcc.dg/bf-ms-layout-3.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/bf-ms-layout-3.c   (with props)
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/stor-layout.c
    trunk/gcc/testsuite/ChangeLog

Propchange: trunk/gcc/testsuite/gcc.dg/bf-ms-layout-3.c
            ('svn:eol-style' added)

Propchange: trunk/gcc/testsuite/gcc.dg/bf-ms-layout-3.c
            ('svn:mime-type' added)

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

--- Comment #10 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-20 22:09:52 UTC ---
Author: ktietz
Date: Mon Feb 20 22:09:48 2012
New Revision: 184410

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184410
Log:
    PR target/52238
    * stor-layout.c (place_field): Handle desired_align for
    ms-bitfields, too.

    * gcc.dg/bf-ms-layout-3.c: New testcase.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/bf-ms-layout-3.c   (with
props)
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/stor-layout.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog

Propchange: branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/bf-ms-layout-3.c
            ('svn:eol-style' added)

Propchange: branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/bf-ms-layout-3.c
            ('svn:mime-type' added)

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #11 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-20 22:18:33 UTC ---
I don't intend to back-port this patch to 4.5, or to 4.4.  Therefore close
report

[Bug target/52238] -mms-bitfields: __attribute__ ((aligned (n))) ignored for struct members

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52238

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jojelino at gmail dot com

--- Comment #12 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-02-22 16:28:40 UTC ---
*** Bug 50057 has been marked as a duplicate of this bug. ***