Bug 37135 - [4.3/4.4 Regression] code size increase for struct assignment
Summary: [4.3/4.4 Regression] code size increase for struct assignment
Status: RESOLVED DUPLICATE of bug 22141
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.3.1
: P2 normal
Target Milestone: 4.3.4
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2008-08-15 21:20 UTC by etienne_lorrain
Modified: 2009-11-25 09:40 UTC (History)
10 users (show)

See Also:
Host: i386-linux-debian
Target: i386-linux-debian
Build: debian
Known to work:
Known to fail:
Last reconfirmed: 2009-02-03 14:23:13


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description etienne_lorrain 2008-08-15 21:20:47 UTC
file test.c:
----------------------
enum stdcolor_enum {
    black,    blue,         green,      cyan,
    red,      magenta,      brown,      lightgray,
    darkgray, lightblue,    lightgreen, lightcyan,
    lightred, lightmagenta, yellow,     white
    };

union mouse_color_union {
    struct mouse_color_str {
        unsigned deflt              : 4;
        unsigned leftbutton         : 4;
        unsigned rightbutton        : 4;
        unsigned middlebutton       : 4;
        unsigned topbutton          : 4;
        unsigned twobutton          : 4;
        unsigned invalid            : 4;
        unsigned infield            : 4;
        } colors;
    unsigned all;
    } conf;

const union mouse_color_union MOUSE_reset_colors = { .colors = {
    .deflt =            brown,
    .leftbutton =       red,
    .rightbutton =      green,
    .middlebutton =     lightcyan,
    .topbutton =        cyan,
    .twobutton =        magenta,
    .invalid =          black,
    .infield =          magenta,
    }};

void fct (void)
{
    conf = MOUSE_reset_colors;
}
----------------------
$ gcc-4.2 --version
gcc-4.2 (GCC) 4.2.4 (Debian 4.2.4-3)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc-4.2 -Os -c test.c -o test.o-4.2
$ size test.o-4.2
   text	   data	    bss	    dec	    hex	filename
     19	      0	      0	     19	     13	test.o-4.2
$ gcc-4.3 --version
gcc-4.3 (Debian 4.3.1-9) 4.3.1
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc-4.3 -Os -c test.c -o test.o-4.3
$ size test.o-4.3
   text	   data	    bss	    dec	    hex	filename
     56	      0	      0	     56	     38	test.o-4.3

Basically, the assembler lines in gcc-4.2:
        movl    MOUSE_reset_colors, %eax
        movl    %eax, conf
becomes in 4.3:
        movl    $0, conf
        movb    conf+3, %al
        movb    $70, conf
        andl    $15, %eax
        orl     $80, %eax
        movb    $-78, conf+1
        movb    $83, conf+2
        movb    %al, conf+3
which looks like a regression.

Thanks, etienne.
Comment 1 Andrew Pinski 2008-08-15 21:26:55 UTC
The gimplifier is inlining the value of MOUSE_reset_colors for some reason ...

Confirmed.
Comment 2 etienne_lorrain 2008-08-17 19:31:26 UTC
 It is not related to bitfields, this also show the problem:
struct color { unsigned char red, green, blue, transparent; } cur_color;
static const struct color mycolor = { 200, 10, 30, 0 };
void fct(void)
{
    cur_color = mycolor;
}
 with gcc-4.3.1 cur_color is initialised with 4 "movb" (so address is encoded 4 times)
 and with gcc-4.2 with a 32 bits constant and a single "movl".

 It seems to be the old problem of gcc not having a "write combining" pass
(converting consecutive byte/word writes into single double word writes), and
the "fix" of defining an intermediate variable is no more working.
Comment 3 etienne_lorrain 2008-08-24 09:13:32 UTC
 Moreover, if in the first test.c program, you declare variable "conf" volatile,
the assembly generated contains 8 writes to "conf", and that memory location
is *read* 7 times. If "conf" does not point to memory but to memory mapped I/O
ports (maybe not readable), that is invalid code generated for valid input.
Comment 4 Joseph S. Myers 2008-08-27 22:05:37 UTC
4.3.2 is released, changing milestones to 4.3.3.
Comment 5 Jakub Jelinek 2008-11-21 15:29:02 UTC
4.4 compiles this (at least at -O2) into:
        movb    $70, conf
        movb    $-78, conf+1
        movb    $83, conf+2
        movb    $80, conf+3
so this looks then as a dup of PR22141.  Testcase without the unnecessary
enum etc.:
union U
{
  struct
  {
    unsigned f1 : 4;
    unsigned f2 : 4;
    unsigned f3 : 4;
    unsigned f4 : 4;
    unsigned f5 : 4;
    unsigned f6 : 4;
    unsigned f7 : 4;
    unsigned f8 : 4;
  } f;
  unsigned g;
} u;

const union U def = { .f = { .f1 = 6, .f2 = 4, .f3 = 2, .f4 = 11,
     .f5 = 3, .f6 = 5, .f7 = 0, .f8 = 5 } };

void fn1 (void)
{
  u = def;
}

void fn3 (union U *);
void fn2 (void)
{
  union U v = def;
  fn3 (&v);
}
Comment 6 etienne_lorrain 2008-11-21 16:10:32 UTC
 By trying to declare:
volatile union U u;
 In your Testcase without the unnecessary enum, the "u = def;" is compiled as:
        movl    $0, u

        movl    u, %eax
        andl    $-16, %eax
        orl     $6, %eax
        movl    %eax, u

        movl    u, %eax
        andb    $15, %al
        orl     $64, %eax
        movl    %eax, u

        movl    u, %eax
        andb    $240, %ah
        orb     $2, %ah
        movl    %eax, u

        movl    u, %eax
        andb    $15, %ah
        orb     $176, %ah
        movl    %eax, u

        movl    u, %eax
        andl    $-983041, %eax
        orl     $196608, %eax
        movl    %eax, u

        movl    u, %eax
        andl    $-15728641, %eax
        orl     $5242880, %eax
        movl    %eax, u

        movl    u, %eax
        andl    $268435455, %eax
        orl     $1342177280, %eax
        movl    %eax, u

 By gcc version 4.3.2 (Debian 4.3.2-1) on i486-linux-gnu at -O2.
Comment 7 Jakub Jelinek 2008-11-21 16:21:53 UTC
For structure assignments to volatile structures you really can't have any expectations how many individual stores will be done.  I guess that falls under:
What constitutes an access to an object that has volatile-qualified type is implementation-defined.

Comment 8 etienne_lorrain 2008-11-21 17:45:48 UTC
 The number of writes for that volatile structure may or may not be a problem,
I am more concerned by the number of reads of some memory which may not be readable at all.
Comment 9 Jakub Jelinek 2008-11-21 22:17:00 UTC
Then don't use bitfields...
Comment 10 Jakub Jelinek 2008-11-24 12:59:04 UTC
Subject: Bug 37135

Author: jakub
Date: Mon Nov 24 12:57:37 2008
New Revision: 142157

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142157
Log:
	PR middle-end/37135
	* dse.c (find_shift_sequence): Optimize extraction from a constant.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dse.c

Comment 11 etienne_lorrain 2008-11-24 22:01:14 UTC
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142157
> Modified: trunk/gcc/dse.c

 I am using gcc-core-4.4-20081121.tar.bz2 with ia32-linux (Fedora9) and
applying your patch gives a better assembly, but still not the possible
"mov $0x12345678,address" - I still get 4 movb on my test.c or your testcase.
 Maybe I shall get the latest SVN, or there is more files to patch than dse.c?
$ size test.o-4.4
   text	   data	    bss	    dec	    hex	filename
     37	      0	      0	     37	     25	test.o-4.4

Comment 12 Richard Biener 2009-01-24 10:20:44 UTC
GCC 4.3.3 is being released, adjusting target milestone.
Comment 13 Paolo Bonzini 2009-02-03 14:23:13 UTC
Adjusting subject, the testcase from comment #2 still produces suboptimal code.
Comment 14 Paolo Bonzini 2009-02-03 14:30:09 UTC
what remains is a dup of 22141.

*** This bug has been marked as a duplicate of 22141 ***
Comment 15 Jakub Jelinek 2009-11-25 09:40:17 UTC
*** Bug 42172 has been marked as a duplicate of this bug. ***