Bug 32901 - [4.1/4.2/4.3 Regression] bitfield constants with multiple bitfields take up space in .data
Summary: [4.1/4.2/4.3 Regression] bitfield constants with multiple bitfields take up s...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.1.2
: P2 normal
Target Milestone: 4.1.3
Assignee: Aldy Hernandez
URL:
Keywords:
Depends on: 34448 34585
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-26 08:35 UTC by Joerg Wunsch
Modified: 2008-01-04 14:18 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 3.4.4
Known to fail: 4.1.2 4.3.0
Last reconfirmed: 2007-08-22 17:16:01


Attachments
Test file (172 bytes, text/plain)
2007-07-26 08:38 UTC, Joerg Wunsch
Details
Result on i386 target from GCC 3.4.4 (174 bytes, text/plain)
2007-07-26 08:39 UTC, Joerg Wunsch
Details
Result on i386 target from GCC 4.1.2 (234 bytes, text/plain)
2007-07-26 08:40 UTC, Joerg Wunsch
Details
Result on avr target from GCC 4.1.2 (310 bytes, text/plain)
2007-07-26 08:41 UTC, Joerg Wunsch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joerg Wunsch 2007-07-26 08:35:04 UTC
When defining a bitfield constant where multiple bitfields have initializing
values, this constant is moved into .data in GCC 4.1.  GCC 3.x could realize
it can be written and assigned as a single integer number.  GCC 4.x only
realizes this situation as long as a single bitfield is initialized.

Verified on i386 and avr targets, so this is apparently completely independent
of the target CPU.
Comment 1 Joerg Wunsch 2007-07-26 08:38:49 UTC
Created attachment 13982 [details]
Test file

Test case.  Compile with -Os -S, and optionally -DONLY_ONE_BITFIELD
to see the difference.
Comment 2 Joerg Wunsch 2007-07-26 08:39:44 UTC
Created attachment 13983 [details]
Result on i386 target from GCC 3.4.4
Comment 3 Joerg Wunsch 2007-07-26 08:40:13 UTC
Created attachment 13984 [details]
Result on i386 target from GCC 4.1.2
Comment 4 Joerg Wunsch 2007-07-26 08:41:09 UTC
Created attachment 13985 [details]
Result on avr target from GCC 4.1.2
Comment 5 Eric Weddington 2007-08-22 17:16:01 UTC
Confirmed on the AVR target for 4.3.0 20070817 snapshot.
Comment 6 Aldy Hernandez 2007-12-04 19:30:13 UTC
Ok, here's the deal.

gimplify_init_constructor promotes the constructor to a static because it is a const aggregate variable, here:

          /* If a const aggregate variable is being initialized, then it        
             should never be a lose to promote the variable to be static.  */
          if (valid_const_initializer
          ...

This isn't a good idea if we are initializing bitfields from static numbers.  If I keep the compiler from promoting to static in the above code, the gimplifier splits the constructor into independent assignments:

  init = {};
  init.a1 = 1;
  init.a2 = 5;

...which later combine munges into the expected constant, thus fixing the problem reported.

Perhaps we should keep the above code from executing if initializing a structure containing only bitfields that fit into a HOST_WIDE_INT, something we can do with one move?  Or is there a better generalization we can code here?  Any suggestions?
Comment 7 Jakub Jelinek 2007-12-04 19:58:13 UTC
The problem is that we really don't have any optimization which would before
expand merge all the sets to adjacent bitfield fields into one (or few) BITFIELD_REFs.  If we are lucky and the RTL passes merge it together, initializing small structs containing bitfields this way will be a win, but
it might be an pessimization as well.
BTW, shouldn't TER try to merge several consecutive initializations of fields
of one aggregate into a CONSTRUCTOR again, so that RTL expanders can do better
job on it?  This wouldn't be the only bug which could be helped by that.
Comment 8 Jakub Jelinek 2007-12-04 20:02:45 UTC
E.g. for PR22141 it would help too (for PR32901 only if the constructor gimplification heuristics is also adjusted).
Comment 9 Aldy Hernandez 2007-12-04 23:43:25 UTC
TER would never see the consecutive initializations because the code I pointed out in gimplify_init_constructor() keeps the gimplifier from splitting up the constructor into consecutive MODIFY_EXPRs.

After gimplification we end up with:

setup_foo ()
{
  static const struct foo init = {.a1=1, .a2=5};
  thefoo = init;
}

So there's nothing to merge back.

What I'm suggesting *is* splitting this up, so subsequent passes can take a stab at it.  In this case, combine cleans it up.
Comment 10 Aldy Hernandez 2007-12-04 23:54:08 UTC
(In reply to comment #8)
> E.g. for PR22141 it would help too (for PR32901 only if the constructor
> gimplification heuristics is also adjusted).
> 

Ok, I see what you mean.  We adjust the heuristic as I've suggested, and write some pass to merge consecutive bitfield stores.  I wonder what pass we can hijack and add the smarts to, instead of writing a whole new pass...
Comment 11 Aldy Hernandez 2007-12-11 19:04:25 UTC
testing a patch