> > This smells bad, since it is given a canonical type that is after the
> > structural equivalency merging that ignores BINFOs, so it may be completely
> > different class with completely different bases than the original. Bases
> > are
> > structuraly merged, too and may be exchanged for normal fields because
> > DECL_ARTIFICIAL (that separate bases and fields) does not seem to be part of
> > the canonical type definition in LTO.
>
> Can you elaborate on that DECL_ARTIFICIAL thing? That is, what is broken
> by considering all fields during that merging?
To make the code work with LTO, one can not merge
struct B {struct A a}
struct B: A {}
these IMO differ only by DECL_ARTIFICIAL flag on the fields.
>
> Note that the BINFO walk below only adds extra aliasing - it should
> be harmless correctness-wise, no?
If it is needed for the second case, then it we will produce wrong code with LTO
when we merge first and second case toghetr. If it is not needed, then we are
safe
but we don't really need the loop then.
Having the testcase where the BINFO walk is needed for corectness, I can turn
it into wrong code in LTO by interposing the class by structure.
I am rebuilding firefox with sanity checking that the second loop never adds
anything
useful. Lets see.
>
> > I wonder if that code is needed after all:
> > case QUAL_UNION_TYPE:
> > /* Recursively record aliases for the base classes, if there are any.
> > */
> > if (TYPE_BINFO (type))
> > {
> > int i;
> > tree binfo, base_binfo;
> >
> > for (binfo = TYPE_BINFO (type), i = 0;
> > BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> > record_alias_subset (superset,
> > get_alias_set (BINFO_TYPE (base_binfo)));
> > }
> > for (field = TYPE_FIELDS (type); field != 0; field = DECL_CHAIN
> > (field))
> > if (TREE_CODE (field) == FIELD_DECL && !DECL_NONADDRESSABLE_P
> > (field))
> > record_alias_subset (superset, get_alias_set (TREE_TYPE (field)));
> > break;
> > all bases are also fields of within the type, so the second loop should
> > notice
> > all the types seen by first loop if I am correct?
> > So perhaps the loop can be dropped at first place.
>
> Yeah, I remember seeing that code and thinking the same multiple times.
> Though I also vaguely remember that removing that loop regressed
> something. How is virtual inheritance represented in the fields?
struct A {int a;};
struct B: virtual A {};
struct C: virtual A {};
struct D: B, C {};
struct D *d;
struct B *b;
int t(void)
{
d->a=1;
return b->a;
}
I srepresented as:
<record_type 0x7ffff6c58000 D addressable needs-constructing type_5 BLK
size <integer_cst 0x7ffff6ae98c0 type <integer_type 0x7ffff6ae7150
bitsizetype> constant 192>
unit size <integer_cst 0x7ffff6ae9880 type <integer_type 0x7ffff6ae70a8
sizetype> constant 24>
align 64 symtab 0 alias set 6 canonical type 0x7ffff6c58000
fields <field_decl 0x7ffff6c4ae40 D.2249
type <record_type 0x7ffff6c452a0 B addressable needs-constructing
type_5 BLK
size <integer_cst 0x7ffff6ae9140 constant 128>
unit size <integer_cst 0x7ffff6ae9160 constant 16>
align 64 symtab 0 alias set 3 canonical type 0x7ffff6c452a0 fields
<field_decl 0x7ffff6c4a2f8 _vptr.B> context <translation_unit_decl
0x7ffff6af1170 D.1>
full-name "struct B"
needs-constructor X() X(constX&) this=(X&) n_parents=1
use_template=0 interface-unknown
pointer_to_this <pointer_type 0x7ffff6c58b28> chain <type_decl
0x7ffff6c3ea10 B>>
ignored decl_6 BLK file t.C line 4 col 8
size <integer_cst 0x7ffff6ae90c0 constant 64>
unit size <integer_cst 0x7ffff6ae90e0 constant 8>
align 64 offset_align 128
offset <integer_cst 0x7ffff6ae9100 constant 0>
bit offset <integer_cst 0x7ffff6ae9180 constant 0> context <record_type
0x7ffff6c58000 D>
chain <field_decl 0x7ffff6c4aed8 D.2250 type <record_type
0x7ffff6c45b28 C>
ignored decl_6 BLK file t.C line 4 col 8 size <integer_cst
0x7ffff6ae90c0 64> unit size <integer_cst 0x7ffff6ae90e0 8>
align 64 offset_align 128 offset <integer_cst 0x7ffff6ae9100 0> bit
offset <integer_cst 0x7ffff6ae90c0 64> context <record_type 0x7ffff6c58000 D>
chain <type_decl 0x7ffff6c590b8 D>>> context <translation_unit_decl
0x7ffff6af1170 D.1>
full-name "struct D"
needs-constructor X() X(constX&) this=(X&) n_parents=2 use_template=0
interface-unknown
pointer_to_this <pointer_type 0x7ffff6c58a80> chain <type_decl
0x7ffff6c59000 D>>
You my end up with A being a field, too, but that only bypas the recursion.
>
> But I'd be happy if this BINFO user would go away ;)
Me too, indeed.
>
> (similar in general for the get_alias_set langhook - with LTO we
> only preserve extra alias-set zero answers from that)
I see, I was surprised we construct alias sets at LTO time, I believed we
always stream them in&out.
Honza
>
> Richard.
