On Thu, 20 Jun 2019, Jason Merrill wrote:

> On Wed, Jun 19, 2019 at 2:47 PM Nathan Sidwell <nat...@acm.org> wrote:
> >
> > On 6/19/19 1:53 PM, Jan Hubicka wrote:
> > >>>> -    ctype = CLASSTYPE_AS_BASE (ctype);
> > >>>> +    {
> > >>>> +      if (!tree_int_cst_equal (TYPE_SIZE (ctype),
> > >>>> +                         TYPE_SIZE (CLASSTYPE_AS_BASE (ctype))))
> > >>>> +        ctype = CLASSTYPE_AS_BASE (ctype);
> > >>>> +    }
> > >>>>      tree clobber = build_clobber (ctype);
> > >>
> > >> I have noticed we build a distinct as-base type in rather more cases than
> > >> strictly necessary.  For instance when there's a member of reference 
> > >> type or
> > >> we have a non-trivial dtor. (CLASSTYPE_NON_LAYOUT_POD_P gets set by a 
> > >> bunch
> > >> of things that don't affect ABI layout)
> > >
> > > Avoiding the extra copies at first place would be great. In my
> > > understanding the types differ by virtual bases and also by their size
> > > since the fake types are not padded to multiply of their alignment.
> > > I guess this can be tested ahead of producing the copy and saving some
> > > memory...
> > >
> > > I am not sure if my C++ FE abilities are on par to implement this tough.
> >
> > I don't think it's simple to fix there, just unfortunate.  your
> > understanding is correct, and I think your workaround will work.
> > However, remember it's possible for T == CLASSTYPE_AS_BASE (T), so might
> > be worth checking that before doing the size comparison?
> >
> > It'd be great to comment on why you're not just using classtype_as_base
> > there.  I suppose I'm serializing this stuff too, with the same
> > inefficiencies ...
> 
> 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'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.

Regular derived classes would simply derive from the as-base type
(as they do now I guess).

Richard.

Reply via email to