Confirmed.

In get_bit_range() we get the following COMPONENT_REF:

(gdb) call debug_generic_stmt(exp)
MEM[(struct io *)0B].e0;

Then we extract the field so we can search for it in the original record/structure:

  field = TREE_OPERAND (exp, 1);
  record_type = DECL_FIELD_CONTEXT (field);

But when we iterate through TYPE_FIELDS(record_type), "field" is not found because the COMPONENT_REF's field "e0" has a different pointer than the "e0" in the record itself:

(gdb) call debug_generic_stmt (fld)
e0

(gdb) call debug_generic_stmt (field)
e0

(gdb) p fld == field
$12 = 0

I'm not very LTO savvy.  Is this supposed to happen?  Should we have been comparing something else than pointer equality here, or is there something else broken?

On Tue, 14 Feb 2012, aldyh at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52097
> 
> Aldy Hernandez <aldyh at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |rguenth at gcc dot gnu.org
> 
> --- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2012-02-14 01:10:43 UTC ---
> In get_bit_range() we get the following COMPONENT_REF:
> 
> (gdb) call debug_generic_stmt(exp)
> MEM[(struct io *)0B].e0;
> 
> Then we extract the field so we can search for it in the original
> record/structure:
> 
>   field = TREE_OPERAND (exp, 1);
>   record_type = DECL_FIELD_CONTEXT (field);
> 
> But when we iterate through TYPE_FIELDS(record_type), "field" is not found
> because the COMPONENT_REF's field "e0" has a different pointer than the "e0" in
> the record itself:
> 
> (gdb) call debug_generic_stmt (fld)
> e0
> 
> (gdb) call debug_generic_stmt (field)
> e0
> 
> (gdb) p fld == field
> $12 = 0
> 
> I'm not very LTO savvy.  Is this supposed to happen?  Should we have been
> comparing something else than pointer equality here, or is there something else
> broken?

Comparing the FIELD_DECL by pointer equality is indeed broken (but
it happens to work without LTO).  The type/IL checkers allow any
FIELD_DECL that is type/position compatible with the one in the record.
Now - in this simple testcase we merge the unnamed main variants of

typedef struct { unsigned int e0 : 16; } s1;

and

typedef struct { unsigned int e0 : 16; } s2;

because they are the same, which causes the variant lists (that contain
s1 for one case and s2 for the other case) to be merged.  Now, those
variants still point to the original FIELD_DECL lists - for one
variant it happens to be the "correct" one, for the other it's "wrong"
(well, "wrong" if you assume that TYPE_FIELDs is shared amongst all
type variants).  I suppose we _can_ fix this in the type merging
process (and that would even save some memory which is good).
So I'll poke at this from the LTO side.

Btw, my C++ memory model bitfield handling rewrite also fixes this bug.

Richard.

Created attachment 26657 [details]
patch

Patch I am going to test.

It works apart from introducing

FAIL: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o link, -O
0 -flto -flto-partition=none -fuse-linker-plugin (internal compiler error)

again (see PR45586).  Let's queue this for 4.8 instead.

Would be interesting to see what the attached patch does to memory usage
with Mozilla though (the Fortran issue is really latent, still changing this
during stage4 seems risky at least - SPEC 2k6 survived a LTO build/run though).

(In reply to comment #6)
> Would be interesting to see what the attached patch does to memory usage
> with Mozilla though (the Fortran issue is really latent, still changing this
> during stage4 seems risky at least - SPEC 2k6 survived a LTO build/run though).

Not much difference in my testing (PGO+LTO Firefox built):
without patch: 
2.5 GB written out
~4.5 GB memory usage (5GB peak)

with your patch:
2.4 GB written out
memory usage roughly the same as before.

Author: rguenth
Date: Tue Mar  6 09:54:06 2012
New Revision: 184981

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184981
Log:
2012-03-06  Richard Guenther  <rguenther@suse.de>

	PR lto/52097
	* lto.c (uniquify_nodes): Merge TYPE_FIELDS of variant types.

	* gcc.dg/lto/pr52097_0.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/lto/pr52097_0.c
Modified:
    trunk/gcc/lto/ChangeLog
    trunk/gcc/lto/lto.c
    trunk/gcc/testsuite/ChangeLog

Fixed, for 4.8.

(In reply to comment #8)
> Author: rguenth
> Date: Tue Mar  6 09:54:06 2012
> New Revision: 184981

This patch seems to have introduced the following testsuite fails on s390x:

                === gfortran tests ===


Running target unix
FAIL: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o link, -O0 -flto -flto-partition=none  (internal compiler error)
UNRESOLVED: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o execute -O0 -flto -flto-partition=none 
FAIL: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o link, -O0 -flto -flto-partition=1to1  (internal compiler error)
UNRESOLVED: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o execute -O0 -flto -flto-partition=1to1 
FAIL: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o link, -O0 -flto  (internal compiler error)
UNRESOLVED: gfortran.dg/lto/pr45586-2 f_lto_pr45586-2_0.o-f_lto_pr45586-2_0.o execute -O0 -flto 
FAIL: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o-f_lto_pr45586_0.o link, -O0 -flto -flto-partition=none  (internal compiler error)
UNRESOLVED: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o-f_lto_pr45586_0.o execute -O0 -flto -flto-partition=none 
FAIL: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o-f_lto_pr45586_0.o link, -O0 -flto -flto-partition=1to1  (internal compiler error)
UNRESOLVED: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o-f_lto_pr45586_0.o execute -O0 -flto -flto-partition=1to1 
FAIL: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o-f_lto_pr45586_0.o link, -O0 -flto  (internal compiler error)
UNRESOLVED: gfortran.dg/lto/pr45586 f_lto_pr45586_0.o-f_lto_pr45586_0.o execute -O0 -flto

(In reply to comment #10)
> (In reply to comment #8)
> > Author: rguenth
> > Date: Tue Mar  6 09:54:06 2012
> > New Revision: 184981
> 
> This patch seems to have introduced the following testsuite fails on s390x:

The PR for that testcase was reopened.

Author: rguenth
Date: Wed Mar 14 10:55:09 2012
New Revision: 185379

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185379
Log:
2012-03-14  Richard Guenther  <rguenther@suse.de>

	* tree.h (DECL_BIT_FIELD_REPRESENTATIVE): New define.
	* stor-layout.c (start_bitfield_representative): New function.
	(finish_bitfield_representative): Likewise.
	(finish_bitfield_layout): Likewise.
	(finish_record_layout): Call finish_bitfield_layout.
	* tree.c (free_lang_data_in_decl): Only free DECL_QUALIFIER
	for QUAL_UNION_TYPE fields.
	* tree-streamer-in.c (lto_input_ts_field_decl_tree_pointers):
	Stream DECL_BIT_FIELD_REPRESENTATIVE.
	* tree-streamer-out.c (write_ts_field_decl_tree_pointers): Likewise.

	PR middle-end/52080
	PR middle-end/52097
	PR middle-end/48124
	* expr.c (get_bit_range): Unconditionally extract bitrange
	from DECL_BIT_FIELD_REPRESENTATIVE.
	(expand_assignment): Adjust call to get_bit_range.

	* gcc.dg/torture/pr48124-1.c: New testcase.
	* gcc.dg/torture/pr48124-2.c: Likewise.
	* gcc.dg/torture/pr48124-3.c: Likewise.
	* gcc.dg/torture/pr48124-4.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-2.c
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-3.c
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c
    trunk/gcc/stor-layout.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-streamer-in.c
    trunk/gcc/tree-streamer-out.c
    trunk/gcc/tree.c
    trunk/gcc/tree.h

Author: rguenth
Date: Mon Jun  4 08:43:23 2012
New Revision: 188167

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188167
Log:
2012-06-04  Richard Guenther  <rguenther@suse.de>
	Eric Botcazou  <ebotcazou@adacore.com>

	Backport from mainline
	2012-04-03  Eric Botcazou  <ebotcazou@adacore.com>

        * expr.c (get_bit_range): Add OFFSET parameter and adjust BITPOS.
        Change type of BITOFFSET to signed.  Make sure the lower bound of
        the computed range is non-negative by adjusting OFFSET and BITPOS.
        (expand_assignment): Adjust call to get_bit_range.

	2012-03-27  Eric Botcazou  <ebotcazou@adacore.com>

        * expr.c (get_bit_range): Return the null range if the enclosing record
        is part of a larger bit field.

	2012-03-20  Richard Guenther  <rguenther@suse.de>

        * stor-layout.c (finish_bitfield_representative): Fallback
        to conservative maximum size if the padding up to the next
        field cannot be computed as a constant.
        (finish_bitfield_layout): If we cannot compute the distance
        between the start of the bitfield representative and the
        bitfield member start a new representative.
        * expr.c (get_bit_range): The distance between the start of
        the bitfield representative and the bitfield member is zero
        if the field offsets are not constants.

	2012-03-16  Richard Guenther  <rguenther@suse.de>

        * stor-layout.c (finish_bitfield_representative): Fall back
        to the conservative maximum size if we cannot compute the
        size of the tail padding.

	2012-03-14  Richard Guenther  <rguenther@suse.de>

	* tree.h (DECL_BIT_FIELD_REPRESENTATIVE): New define.
	* stor-layout.c (start_bitfield_representative): New function.
	(finish_bitfield_representative): Likewise.
	(finish_bitfield_layout): Likewise.
	(finish_record_layout): Call finish_bitfield_layout.
	* tree.c (free_lang_data_in_decl): Only free DECL_QUALIFIER
	for QUAL_UNION_TYPE fields.
	* tree-streamer-in.c (lto_input_ts_field_decl_tree_pointers):
	Stream DECL_BIT_FIELD_REPRESENTATIVE.
	* tree-streamer-out.c (write_ts_field_decl_tree_pointers): Likewise.

	PR middle-end/52080
	PR middle-end/52097
	PR middle-end/48124
	* expr.c (get_bit_range): Unconditionally extract bitrange
	from DECL_BIT_FIELD_REPRESENTATIVE.
	(expand_assignment): Adjust call to get_bit_range.

	* gcc.dg/torture/pr48124-1.c: New testcase.
	* gcc.dg/torture/pr48124-2.c: Likewise.
	* gcc.dg/torture/pr48124-3.c: Likewise.
	* gcc.dg/torture/pr48124-4.c: Likewise.
	* gnat.dg/pack16.adb: Likewise.
	* gnat.dg/pack16_pkg.ads: Likewise.
	* gnat.dg/pack17.adb: Likewise.
	* gnat.dg/specs/pack7.ads: Likewise.
	* gnat.dg/specs/pack8.ads: Likewise.
	* gnat.dg/specs/pack8_pkg.ads: Likewise.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-1.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-2.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-3.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-4.c
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack16.adb
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack16_pkg.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack17.adb
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack7.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack8.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack8_pkg.ads
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/expr.c
    branches/gcc-4_7-branch/gcc/stor-layout.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-streamer-in.c
    branches/gcc-4_7-branch/gcc/tree-streamer-out.c
    branches/gcc-4_7-branch/gcc/tree.c
    branches/gcc-4_7-branch/gcc/tree.h