> > > 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.
The code really seems to be adding only cases of zero sized classes. I use the
following hack in my tree. I do not know how to discover zero sized class, so I
test of unit size 1, but I think if there was other cases, we would notice
anyway.
The reason why I am looking into is that because I am trying to evaulate how
expensive BINFOs are. I am trying to keep only those that matters for debug
info and devirt,
and avoid streaming others.
Honza
Index: alias.c
===================================================================
--- alias.c (revision 207777)
+++ alias.c (working copy)
@@ -995,20 +996,40 @@ record_component_aliases (tree type)
case RECORD_TYPE:
case UNION_TYPE:
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)));
+ {
+#ifdef ENABLE_CHECKING
+ bitmap_obstack my_obstack;
+ bitmap_obstack_initialize (&my_obstack);
+ bitmap added = BITMAP_ALLOC (&my_obstack);
+#endif
+ alias_set_type t;
+ for (field = TYPE_FIELDS (type); field != 0; field = DECL_CHAIN (field))
+ if (TREE_CODE (field) == FIELD_DECL && !DECL_NONADDRESSABLE_P (field))
+ {
+ t = get_alias_set (TREE_TYPE (field));
+#ifdef ENABLE_CHECKING
+ bitmap_set_bit (added, t);
+#endif
+ record_alias_subset (superset, t);
+ }
+#ifdef ENABLE_CHECKING
+ /* Recursively record aliases for the base classes, if there are any.
*/
+ if (!in_lto_p && TYPE_BINFO (type))
+ {
+ int i;
+ tree binfo, base_binfo;
+
+ for (binfo = TYPE_BINFO (type), i = 0;
+ BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ if (!bitmap_bit_p (added, get_alias_set (BINFO_TYPE
(base_binfo))))
+ {
+ gcc_assert (integer_onep (TYPE_SIZE_UNIT (BINFO_TYPE
(base_binfo))));
+ }
+ }
+ BITMAP_FREE (added);
+ bitmap_obstack_release (&my_obstack);
+#endif
+ }
break;
case COMPLEX_TYPE:
