> > 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?
>
> We were also missing to compare DECL_ABSTRACT_ORIGIN during
> tree merging (not sure if that caused any issue, I just run
> into the clone abstract origin issue above and saw that).
>
> LTO bootstrapped and tested on x86_64-unknown-linux-gnu, I'll throw
> it on regular bootstrap & test and then plan to commit it unless
> you object that lto-symtab.c hunk...
>
> Richard.
>
> 2015-08-31 Richard Biener <rguent...@suse.de>
>
> lto/
> * lto.c (compare_tree_sccs_1): Compare DECL_ABSTRACT_ORIGIN.
> * lto-symtab.c (lto_symtab_merge): Merge DECL_POSSIBLY_INLINED flag.
> (lto_symtab_prevailing_decl): Do not replace a decl that didn't
> participate in merging with something else.
>
> Index: gcc/lto/lto.c
> ===================================================================
> --- gcc/lto/lto.c (revision 227339)
> +++ gcc/lto/lto.c (working copy)
> @@ -1305,6 +1305,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> compare_tree_edges (DECL_SIZE (t1), DECL_SIZE (t2));
> compare_tree_edges (DECL_SIZE_UNIT (t1), DECL_SIZE_UNIT (t2));
> compare_tree_edges (DECL_ATTRIBUTES (t1), DECL_ATTRIBUTES (t2));
> + compare_tree_edges (DECL_ABSTRACT_ORIGIN (t1), DECL_ABSTRACT_ORIGIN
> (t2));
> if ((code == VAR_DECL
> || code == PARM_DECL)
> && DECL_HAS_VALUE_EXPR_P (t1))
> Index: gcc/lto/lto-symtab.c
> ===================================================================
> --- gcc/lto/lto-symtab.c (revision 227339)
> +++ gcc/lto/lto-symtab.c (working copy)
> @@ -312,6 +312,11 @@ lto_symtab_merge (symtab_node *prevailin
>
> if (TREE_CODE (decl) == FUNCTION_DECL)
> {
> + /* Merge decl state in both directions, we may still end up using
> + the new decl. */
> + DECL_POSSIBLY_INLINED (prevailing_decl) |= DECL_POSSIBLY_INLINED
> (decl);
> + DECL_POSSIBLY_INLINED (decl) |= DECL_POSSIBLY_INLINED
> (prevailing_decl);
> +
> if (warn_type_compatibility_p (TREE_TYPE (prevailing_decl),
> TREE_TYPE (decl)))
> return false;
> @@ -798,6 +803,18 @@ lto_symtab_prevailing_decl (tree decl)
> if (TREE_CODE (decl) == FUNCTION_DECL && DECL_ABSTRACT_P (decl))
> return decl;
>
> + /* When decl did not participate in symbol resolution leave it alone.
> + This can happen when we streamed the decl as abstract origin
> + from the block tree of inlining a partially inlined function.
> + If all, the split function and the original function end up
> + optimized away early we do not put the abstract origin into the
> + ltrans boundary and we'll end up ICEing in
> + dwarf2out.c:gen_inlined_subroutine_die because we eventually
> + replace a decl with DECL_POSSIBLY_INLINED set with one without. */
> + if (TREE_CODE (decl) == FUNCTION_DECL
> + && ! cgraph_node::get (decl))
> + return decl;
So here you care about not merging DECLs that are not part of symtab with
decls that are part of symtab? I have bit hard time to understand why that
helps.
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.
Honza
