[Bug target/57344] New: wrong code with pragma pack(1) and -O1 on x86

[Bug target/57344] New: wrong code with pragma pack(1) and -O1 on x86

[dhazeghi at yahoo dot com]

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

            Bug ID: 57344
           Summary: wrong code with pragma pack(1) and -O1 on x86
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dhazeghi at yahoo dot com

The following code produces wrong output on x86_64-linux-gnu in 32-bit mode,
when compiled at -O1 or higher optimizations.  It appears that the behavior
changed between gcc 4.6 which works at all optimization levels, and 4.7 which
does not.

$ gcc-trunk -v
gcc version 4.9.0 20130520 (experimental) [trunk revision 199099] (GCC) 
$ gcc-trunk -O0 -m32 wrong.c 
$ ./a.out 
-3161
$ gcc-trunk -O1 -m64 wrong.c 
$ ./a.out 
-3161
$ gcc-4.6 -O1 -m32 wrong.c 
$ ./a.out 
-3161
$ gcc-trunk -O1 -m32 wrong.c 
$ ./a.out 
2093991
$
--------------------------
int printf(const char *, ...);

#pragma pack(1)
struct S0
{
    int f0:11;
    int f1:22;
    char f2;
    int:0;
}
s[2];
int i;
int
main ()
{
    struct S0 tmp = { 0, -3161L };
    s[1] = tmp;
    for (; i < 1; i++)
        printf ("%d\n", s[1].f1);
    return 0;
}

[Bug middle-end/57344] [4.8/4.9 Regression] wrong code with pragma pack(1) and -O1 on x86

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-05-21
                 CC|                            |jakub at gcc dot gnu.org
          Component|target                      |middle-end
   Target Milestone|---                         |4.8.1
            Summary|wrong code with pragma      |[4.8/4.9 Regression] wrong
                   |pack(1) and -O1 on x86      |code with pragma pack(1)
                   |                            |and -O1 on x86
     Ever confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Weird, bisect tells me this regressed with r186825, but that change wasn't a
trunk change at all.  *.optimized dump is the same, *.expand already looks
buggy.

[Bug rtl-optimization/57344] [4.7/4.8/4.9 Regression] wrong code with pragma pack(1) and -O1 on x86

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |rtl-optimization
      Known to work|                            |4.7.0
   Target Milestone|4.8.1                       |4.7.4
            Summary|[4.8/4.9 Regression] wrong  |[4.7/4.8/4.9 Regression]
                   |code with pragma pack(1)    |wrong code with pragma
                   |and -O1 on x86              |pack(1) and -O1 on x86
      Known to fail|                            |4.7.1, 4.7.3, 4.8.0, 4.9.0

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Bi-secting on the 4.7 branch should reveal the offending patch.  Tree
optimizers
look good, so the issue is on the RTL or target side.

[Bug rtl-optimization/57344] [4.7/4.8/4.9 Regression] wrong code with pragma pack(1) and -O1 on x86

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Yeah, the problem is definitely in store_split_bit_field.

One possible fix is:

--- gcc/expmed.c.jj    2013-05-14 10:54:58.000000000 +0200
+++ gcc/expmed.c    2013-05-21 10:54:59.707793889 +0200
@@ -1094,10 +1094,15 @@ store_split_bit_field (rtx op0, unsigned
       thispos = (bitpos + bitsdone) % unit;

       /* When region of bytes we can touch is restricted, decrease
-     UNIT close to the end of the region as needed.  */
+     UNIT close to the end of the region as needed.
+     Don't do this if op0 is a REG or SUBREG, there won't be any
+     data races on registers and the code assumes unit is
+     BITS_PER_WORD.  */
       if (bitregion_end
       && unit > BITS_PER_UNIT
-      && bitpos + bitsdone - thispos + unit > bitregion_end + 1)
+      && bitpos + bitsdone - thispos + unit > bitregion_end + 1
+      && !REG_P (op0)
+      && GET_CODE (op0) != SUBREG)
     {
       unit = unit / 2;
       continue;

Another one I'm currently testing is just not assuming unit is BITS_PER_WORD in
the SUBREG/REG handling code, and another one a mixture of both.

I don't know if we can actually end up with a SUBREG of MEM, if yes, then the
above patch is wrong, because it could introduce a data race in that case.  The
other fix on the other side might pessimize the case where we e.g. have a real
REG or SUBREG of some REG and store some bits in a first word and bitregion_end
tells us that we must not touch the last byte in the second word, but we need
to touch the rest.  On REGs there should be no data races.  So I'm probably
going with both changes together.

[Bug rtl-optimization/57344] [4.7/4.8/4.9 Regression] wrong code with pragma pack(1) and -O1 on x86

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30151
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30151&action=edit
gcc49-pr57344.patch

Untested fix.

[Bug rtl-optimization/57344] [4.7 Regression] wrong code with pragma pack(1) and -O1 on x86

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.1, 4.9.0
            Summary|[4.7/4.8/4.9 Regression]    |[4.7 Regression] wrong code
                   |wrong code with pragma      |with pragma pack(1) and -O1
                   |pack(1) and -O1 on x86      |on x86
      Known to fail|4.9.0                       |

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu May 23 09:17:34 2013
New Revision: 199238

URL: http://gcc.gnu.org/viewcvs?rev=199238&root=gcc&view=rev
Log:
    PR middle-end/57344
    * expmed.c (store_split_bit_field): If op0 is a REG or
    SUBREG of a REG, don't lower unit.  Handle unit not being
    always BITS_PER_WORD.

    * gcc.c-torture/execute/pr57344-1.c: New test.
    * gcc.c-torture/execute/pr57344-2.c: New test.
    * gcc.c-torture/execute/pr57344-3.c: New test.
    * gcc.c-torture/execute/pr57344-4.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr57344-1.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr57344-2.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr57344-3.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr57344-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expmed.c
    trunk/gcc/testsuite/ChangeLog

Author: jakub
Date: Thu May 23 09:18:57 2013
New Revision: 199239

URL: http://gcc.gnu.org/viewcvs?rev=199239&root=gcc&view=rev
Log:
    PR middle-end/57344
    * expmed.c (store_split_bit_field): If op0 is a REG or
    SUBREG of a REG, don't lower unit.  Handle unit not being
    always BITS_PER_WORD.

    * gcc.c-torture/execute/pr57344-1.c: New test.
    * gcc.c-torture/execute/pr57344-2.c: New test.
    * gcc.c-torture/execute/pr57344-3.c: New test.
    * gcc.c-torture/execute/pr57344-4.c: New test.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr57344-1.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr57344-2.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr57344-3.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr57344-4.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/expmed.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

Fixed for 4.8/4.9+ so far.

[Bug rtl-optimization/57344] [4.7 Regression] wrong code with pragma pack(1) and -O1 on x86

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57344

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|4.7.4                       |4.8.1
      Known to fail|                            |4.7.4

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed in 4.8.1.