Re: TYPE_BINFO and canonical types at LTO

[Jan Hubicka <hubicka at ucw dot cz>]

> 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