Use ODR for canonical types construction in LTO

Jan Hubicka hubicka@ucw.cz
Mon Jun 24 11:39:00 GMT 2019


> On Mon, 24 Jun 2019, Jan Hubicka wrote:
> 
> > > > This simple (untested) patch doesn't avoid creating the unnecessary
> > > > as-base types, but it should avoid using them in a way that causes
> > > > them to be streamed, and should let them be discarded by GC.
> > > > Thoughts?
> > > 
> > > Looks better than Honzas patch fixing a single place.
> > 
> > I wonder if we can go ahead with Jason's patch to handle the common
> > case.
> 
> I hope so - Jason?
> 
> > > 
> > > I've spent some thoughts on this and I wonder whether we can
> > > re-implement classtype-as-base with fake inheritance (which would
> > > also solve the TBAA alias set issue in a natural way).  That is,
> > > we'd lay out structs as-base and make instances of it use a
> > > 
> > > class as-instance { as-base b; X pad1; Y pad2; };
> > > 
> > > with either explicit padding fields or with implicit ones
> > > (I didn't check how we trick stor-layout to not pad the as-base
> > > type to its natural alignment...).
> > > 
> > > I realize that this impacts all code building component-refs ontop
> > > of as-instance typed objects so this might rule out this approach
> > > completely - but maybe that's reasonably well abstracted into common
> > > code so only few places need adjustments.
> > 
> > Modulo the empty virtual bases which I have no understnading to I
> > suppose this should work.
> > 
> > One issue is that we will need to introduce view_convert_exprs at some
> > times.
> > 
> > As
> > 
> >   class a var;
> >   class b:a {} *bptr;
> > 
> >   var.foo;
> > 
> > Expanding this as var.as_base_a.foo would make access path oracle to
> > disambiguate it from bptr->as_base_b->as_base_a.foo which is wrong with
> > gimple memory moel where we allow placement new replacing var by
> > instance of b.
> 
> Ick.  IIRC the as-base types were necessary only for
> copying and clobber operations that may not touch the possibly
> re-used tail-padding.  The question is whether we cannot invent
> a better mechanism to do this -- IIRC we used memmove for the
> former case at some point (that's arguably worse, also for TBAA).
> There's WITH_SIZE_EXPR which we only handle rudimentary in the
> middle-end though.

Yep, only place where as-base type surface to middle-end are clobbers.
call.c does:

        {
          /* We must only copy the non-tail padding parts.  */
          tree arg0, arg2, t;
          tree array_type, alias_set;

          arg2 = TYPE_SIZE_UNIT (as_base);
          arg0 = cp_build_addr_expr (to, complain);

          array_type = build_array_type (unsigned_char_type_node,
                                         build_index_type
                                           (size_binop (MINUS_EXPR,
                                                        arg2, size_int (1))));
          alias_set = build_int_cst (build_pointer_type (type), 0);
          t = build2 (MODIFY_EXPR, void_type_node,
                      build2 (MEM_REF, array_type, arg0, alias_set),
                      build2 (MEM_REF, array_type, arg, alias_set));
          val = build2 (COMPOUND_EXPR, TREE_TYPE (to), t, to);
          TREE_NO_WARNING (val) = 1;
        }

I have noticed this earlier too since it disables access path oracle
(TREE_TYPE (mem_ref) is char array, while ptrtype is the original type)
and thus is pretty bad, too. At least it tests if types are different
and therefore this path is relatively rare.

We should solve both these places indeed.  I think call.c could also use
as-base type like clobber codegen if LTO TBAA was fixed on those.

I agree that having solution that does not complicate middle-end would
be nice, but if we want to get as-base types right the way they are now
we need to expose IS_FAKE_BASE_TYPE falg to the middle-end (perhaps with
better name).  Then we can make canonical_type_used_p return false for
those and make get_alias_set and same_type_for_tbaa_p to use
TYPE_CONTEXT (type) in this case.
This ought to get things work quite smoothly.
Since we do not do component refs on those, we should not get into
problem with nonoverlapping_component_refs not considering the types the
same.

Honza



More information about the Gcc-patches mailing list