> On Fri, 14 Feb 2014, Jan Hubicka wrote:
>
> > > > 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.
>
> "The code" == that BINFO walk? Is that because we walk a completely
Yes.
> unrelated BINFO chain? I'd say we should have merged its types
> so that difference shouldn't matter.
>
> Hopefully ;)
I am trying to make point that will matter. Here is completed testcase above:
struct A {int a;};
struct C:A {};
struct B {struct A a;};
struct C *p2;
struct B *p1;
int
t()
{
p1->a.a = 2;
return p2->a;
}
With patch I get:
Index: lto/lto.c
===================================================================
--- lto/lto.c (revision 207777)
+++ lto/lto.c (working copy)
@@ -49,6 +49,8 @@ along with GCC; see the file COPYING3.
#include "data-streamer.h"
#include "context.h"
#include "pass_manager.h"
+#include "print-tree.h"
/* Number of parallel tasks to run, -1 if we want to use GNU Make jobserver.
*/
@@ -619,6 +621,15 @@ gimple_canonical_type_eq (const void *p1
{
const_tree t1 = (const_tree) p1;
const_tree t2 = (const_tree) p2;
+ if (gimple_canonical_types_compatible_p (CONST_CAST_TREE (t1),
+ CONST_CAST_TREE (t2))
+ && TREE_CODE (CONST_CAST_TREE (t1)) == RECORD_TYPE)
+ {
+ debug_tree (CONST_CAST_TREE (t1));
+ fprintf (stderr, "bases:%i\n", BINFO_BASE_BINFOS (TYPE_BINFO
(t1))->length());
+ debug_tree (CONST_CAST_TREE (t2));
+ fprintf (stderr, "bases:%i\n", BINFO_BASE_BINFOS (TYPE_BINFO
(t2))->length());
+ }
return gimple_canonical_types_compatible_p (CONST_CAST_TREE (t1),
CONST_CAST_TREE (t2));
}
<record_type 0x7ffff6c52888 B SI
size <integer_cst 0x7ffff6ae83a0 type <integer_type 0x7ffff6ae5150
bitsizetype> constant 32>
unit size <integer_cst 0x7ffff6ae83c0 type <integer_type 0x7ffff6ae50a8
sizetype> constant 4>
align 32 symtab 0 alias set -1 canonical type 0x7ffff6c52888
fields <field_decl 0x7ffff6adec78 a
type <record_type 0x7ffff6c52738 A SI size <integer_cst 0x7ffff6ae83a0
32> unit size <integer_cst 0x7ffff6ae83c0 4>
align 32 symtab 0 alias set -1 canonical type 0x7ffff6c52738 fields
<field_decl 0x7ffff6adebe0 a> context <translation_unit_decl 0x7ffff6af2e60
D.2821>
chain <type_decl 0x7ffff6af2f18 A>>
nonlocal SI file t.C line 3 col 20 size <integer_cst 0x7ffff6ae83a0 32>
unit size <integer_cst 0x7ffff6ae83c0 4>
align 32 offset_align 128
offset <integer_cst 0x7ffff6ae8060 constant 0>
bit offset <integer_cst 0x7ffff6ae80e0 constant 0> context <record_type
0x7ffff6c52888 B>
chain <type_decl 0x7ffff6c55170 B type <record_type 0x7ffff6c52930 B>
nonlocal VOID file t.C line 3 col 10
align 1 context <record_type 0x7ffff6c52888 B> result <record_type
0x7ffff6c52888 B>>> context <translation_unit_decl 0x7ffff6af2e60 D.2821>
pointer_to_this <pointer_type 0x7ffff6c529d8> chain <type_decl
0x7ffff6c550b8 B>>
bases:0
<record_type 0x7ffff6c52b28 C SI
size <integer_cst 0x7ffff6ae83a0 type <integer_type 0x7ffff6ae5150
bitsizetype> constant 32>
unit size <integer_cst 0x7ffff6ae83c0 type <integer_type 0x7ffff6ae50a8
sizetype> constant 4>
align 32 symtab 0 alias set -1 structural equality
fields <field_decl 0x7ffff6adeda8 D.2831
type <record_type 0x7ffff6c52738 A SI size <integer_cst 0x7ffff6ae83a0
32> unit size <integer_cst 0x7ffff6ae83c0 4>
align 32 symtab 0 alias set -1 canonical type 0x7ffff6c52738 fields
<field_decl 0x7ffff6adebe0 a> context <translation_unit_decl 0x7ffff6af2e60
D.2821>
chain <type_decl 0x7ffff6af2f18 A>>
ignored SI file t.C line 2 col 8 size <integer_cst 0x7ffff6ae83a0 32>
unit size <integer_cst 0x7ffff6ae83c0 4>
align 32 offset_align 128
offset <integer_cst 0x7ffff6ae8060 constant 0>
bit offset <integer_cst 0x7ffff6ae80e0 constant 0> context <record_type
0x7ffff6c52a80 C>
chain <type_decl 0x7ffff6c552e0 C type <record_type 0x7ffff6c52b28 C>
nonlocal VOID file t.C line 2 col 12
align 1 context <record_type 0x7ffff6c52a80 C> result <record_type
0x7ffff6c52a80 C>>> context <translation_unit_decl 0x7ffff6af2e60 D.2821>
chain <type_decl 0x7ffff6c55228 C>>
bases:1
So we prevail structure B with structure C. One has bases to walk other
doesn't.
If that BINFO walk in alias.c (on canonical types) did something useful, we
have a wrong code bug.
Yes, zero sized classes are those having no fields (but other stuff, type
decls, bases etc.)
Honza
