> On Tue, 1 Sep 2015, Jan Hubicka wrote:
>
> > > > Yep, this looks like a resonable direction. It will break the one
> > > > declaration
> > > > rule in a more wild sense than current frontends does so, because if a
> > > > builtin
> > > > win as a prevailing declaration, we end up with no merging at all.
> > > > I wonder if we don't want to always prevail to first non-builtin
> > > > variant?
> > >
> > > I think we already try. But this can be tricky as seen by the following
> > > which is needed to finally fix LTO bootstrap on trunk ...
> > >
> > > Basically there are some function decls that do not go through
> > > lto-symtab handling because they are not in any CUs cgraph but
> > > only exist because they are refered to from BLOCK abstract origins
> > > (and thus also LTRANS boundary streaming doesn't force them
> > > into the cgraph). For example during LTO bootstrap I see
> > > this from inlining of split functions where both split parts and
> > > the original function are optimized away before stream-out.
> >
> > Yep, I remember poking around this when I finally stopped my attempts
> > to get abstract origins working with LTO :(
> > >
> > > We hit
> > >
> > > static void
> > > gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die)
> > > {
> > > ...
> > > /* Make sure any inlined functions are known to be inlineable. */
> > > gcc_checking_assert (DECL_ABSTRACT_P (decl)
> > > || cgraph_function_possibly_inlined_p (decl));
> > >
> > > a lot without the following fixes. I suppose we _can_ merge
> > > function decls which just differ in DECL_POSSIBLY_INLINED if
> > > we make sure to "merge" the flag during tree merging. But
> > > that's for a followup, below is for correctness.
> >
> > How this is going to work with early debug? If we merge one function to
> > another, won't we move the references inside blocks to references to another
> > block tree that is possibly incompatible with one we expect?
>
> Hmm, indeed, if we merge the abstract instance FUNCTION_DECL we end
> up adjusting the abstract origin DIE reference accordingly but as
> we don't merge BLOCKs the actual BLOCK abstract origins would refer
> to the "original" early debug abstract instance. Not sure if that
> will cause problems for the debug consumer - the abstract instances
> should be the same (literally, unless you play tricks with #ifdefs
> in different uses...).
>
> > The rest of patch makes sense to me modulo the possible issues with moving
> > ECL_ABSTRACT_ORIGIN reference to an incompatible blob of dwarf info comming
> > from other instance of the inlined function from other translation unit.
> >
> > I see you added DECL_ABSTRACT_ORIGIN comparer, but that won't make block
> > numbers
> > to match.
>
> Who cares about block numbers? Note the abstract origin comparer was
> for the clone abstract origins we have.
Yep, no one in dwarf2out. My original attempt used it to refer to BLOCKs within
a different function body (BLOCK_ABSTRACT_ORIGIN). We currently don't stream
it, so
we don't need to refer to it.
In longer run we will however need a way to make these references to work...
Merging them will be tricky as it would require comparing the function's block
trees.
Especially after early inlining these tends to diverge, but perhaps the early
debug is output
before early inlining and thus this problem goes away and we only care about
different ifdefs.
Those are quite common, too, however, at least in Firefox :(
Honza
>
> Richard.
