Re: Enable pointer TBAA for LTO

[Richard Biener <rguenther at suse dot de>]

On Mon, 23 Nov 2015, Martin Jambor wrote:

> Hi,
> 
> On Mon, Nov 23, 2015 at 12:00:25AM +0100, Jan Hubicka wrote:
> > Hi,
> > here is updated patch which I finally comitted today.  It addresses all the comments
> > and also fixes one nasty bug that really cost me a lot of time to understand. 
> > 
> > +	  /* LTO type merging does not make any difference between 
> > +	     component pointer types.  We may have
> > +
> > +	     struct foo {int *a;};
> > +
> > +	     as TYPE_CANONICAL of 
> > +
> > +	     struct bar {float *a;};
> > +
> > +	     Because accesses to int * and float * do not alias, we would get
> > +	     false negative when accessing the same memory location by
> > +	     float ** and bar *. We thus record the canonical type as:
> > +
> > +	     struct {void *a;};
> > +
> > +	     void * is special cased and works as a universal pointer type.
> > +	     Accesses to it conflicts with accesses to any other pointer
> > +	     type.  */
> > 
> > This problem manifested itself only as a lto-bootstrap miscompare on 32bit
> > build and I spent a lot of time localizing the wrong code since it reproduces
> > only in quite large programs where we get conficts in canonical type merging
> > like this.
> > 
> > The patch thus updates record_component_aliases to substitute void_ptr_type for
> > all pointer types. I re-did the stats.  Now the improvement on dealII is 14%
> > that is quite a bit lower than earlier, but still substantial.  Since we have
> > voidptr globing counter, I know that the number of disambiguations would go at
> > least 19% up if we did not do it.
> > 
> > THere is a lot of low hanging fruit in that area now, but the real solution is to
> > track types that needs to be merge by fortran rules and don't do all this fancy
> > globing for C/C++ types.  I will open branch for IPA work and try to prepare this
> > for next stage 1.
> > 
> > bootstrapped/regtested x86_64-linux and ppc64-linux, earlier version tested on i386-linux
> > and also on some bigger apps, committed
> > 
> > Note that we still have bootstrap miscompare with LTO build and --disable-checking,
> > I am looking for that now.  Additoinally after fixing the ICEs preventing us to build
> > the gnat1 binary, gnat1 aborts. Both these are independent of the patch.
> > 
> > Honza
> > 	* lto.c (iterative_hash_canonical_type): Always recurse for pointers.
> > 	(gimple_register_canonical_type_1): Check that pointers do not get
> > 	canonical types.
> > 	(gimple_register_canonical_type): Do not register pointers.
> > 
> > 	* tree.c (build_pointer_type_for_mode,build_reference_type_for_mode):
> > 	In LTO we do not compute TYPE_CANONICAL of pointers.
> > 	(gimple_canonical_types_compatible_p): Improve coments; sanity check
> > 	that pointers do not have canonical type that would make us believe
> > 	they are different.
> > 	* alias.c (get_alias_set): Do structural type equality on pointers;
> > 	enable pointer path for LTO; also glob pointer to vector with pointer
> > 	to vector element; glob pointers and references for LTO; do more strict
> > 	sanity checking about build_pointer_type returning the canonical type
> > 	which is also the main variant.
> > 	(record_component_aliases): When component type is pointer and we
> > 	do LTO; record void_type_node alias set.
> 
> ...
> 
> > Index: alias.c
> > ===================================================================
> > --- alias.c	(revision 230714)
> > +++ alias.c	(working copy)
> > @@ -869,13 +869,23 @@ get_alias_set (tree t)
> >        set = lang_hooks.get_alias_set (t);
> >        if (set != -1)
> >  	return set;
> > -      return 0;
> > +      /* Handle structure type equality for pointer types.  This is easy
> > +	 to do, because the code bellow ignore canonical types on these anyway.
> > +	 This is important for LTO, where TYPE_CANONICAL for pointers can not
> > +	 be meaningfuly computed by the frotnend.  */
> > +      if (!POINTER_TYPE_P (t))
> > +	{
> > +	  /* In LTO we set canonical types for all types where it makes
> > +	     sense to do so.  Double check we did not miss some type.  */
> > +	  gcc_checking_assert (!in_lto_p || !type_with_alias_set_p (t));
> > +          return 0;
> 
> I have hit this assert on our LTO tests when doing a merge from trunk
> to the HSA branch.  On the branch, we generate very simple static
> constructors/destructors which just call libgomp (un)registration
> routines to which we pass data in static variables of artificial
> types.  The assert happens inside varpool_node::finalize_decl calls on
> those variables, e.g.:
> 
> lto1: internal compiler error: in get_alias_set, at alias.c:880
> 0x613650 get_alias_set(tree_node*)
>         /home/mjambor/gcc/branch/src/gcc/alias.c:880
> 0x71d2c7 set_mem_attributes_minus_bitpos(rtx_def*, tree_node*, int, long)
>         /home/mjambor/gcc/branch/src/gcc/emit-rtl.c:1772
> 0xd2d2f0 make_decl_rtl(tree_node*)
>         /home/mjambor/gcc/branch/src/gcc/varasm.c:1473
> 0xd310c7 assemble_variable(tree_node*, int, int, int)
>         /home/mjambor/gcc/branch/src/gcc/varasm.c:2144
> 0xd37b32 varpool_node::assemble_decl()
>         /home/mjambor/gcc/branch/src/gcc/varpool.c:580
> 0x6896aa varpool_node::finalize_decl(tree_node*)
>         /home/mjambor/gcc/branch/src/gcc/cgraphunit.c:820
> 0x844da1 hsa_output_kernels
>         /home/mjambor/gcc/branch/src/gcc/hsa-brig.c:1954
> 0x849f04 hsa_output_libgomp_mapping
>         /home/mjambor/gcc/branch/src/gcc/hsa-brig.c:2116
> 0x849f04 hsa_output_brig()
>         /home/mjambor/gcc/branch/src/gcc/hsa-brig.c:2356
> (hsa_output_brig is called from compile_file in toplev.c)

I think it also causes the following and one related ICE

FAIL: gcc.dg/vect/pr62021.c -flto -ffat-lto-objects (internal compiler 
error)

/space/rguenther/src/svn/trunk3/gcc/testsuite/gcc.dg/vect/pr62021.c:7:1: 
internal compiler error: in get_alias_set, at alias.c:880^M
0x7528a7 get_alias_set(tree_node*)^M
        /space/rguenther/src/svn/trunk3/gcc/alias.c:880^M
0x751ce5 component_uses_parent_alias_set_from(tree_node const*)^M
        /space/rguenther/src/svn/trunk3/gcc/alias.c:635^M
0x7522ad reference_alias_ptr_type_1^M
        /space/rguenther/src/svn/trunk3/gcc/alias.c:747^M
0x752683 get_alias_set(tree_node*)^M
...

Richard.