[PATCH] Use aliases instead of separate copies for !DECL_ONE_ONLY C[12] ctors or D[12] dtors with identical bodies (PR c++/3187, take 2)

[Jakub Jelinek]

On Fri, Nov 13, 2009 at 08:19:55PM +0100, Jan Hubicka wrote:
> You will need to update LTO streamer to handle same_body_aliases and
> build the alias nodes.

Ah, LTO, that would mean I need to actually configure it and test :(.

> > +/* Remove same body alias node.  */
> > +
> > +static void
> > +cgraph_remove_same_body_alias (struct cgraph_node *node)
> 
> I guess we are losing bit of precision here by removing the aliases all
> at once only after all possible references to each of them disappear.
> Doing per-alias reachability in cgraph_remove_unreachable_node would be
> probably possible, but bit ugly (we will need to tag cgraph edge with
> extra info to what alias edge goes) and I guess it is not worthwhile at
> all.  Or do you foresee a lot of redundant COMDAT symbol table entries
> created this way?

At least for the cdtor COMDAT aliases I need to output either all of them
(if any of them is needed), or none of them.  Otherwise the COMDAT group
could provide different symbols between different CUs and we'd lose.

> 
> > @@ -1041,7 +1041,12 @@ cgraph_emit_thunks (void)
> >  	 cgraph_mark_functions_to_output in cgraph_optimize).  */
> >        if (n->reachable
> >  	  && !DECL_EXTERNAL (n->decl))
> > -	lang_hooks.callgraph.emit_associated_thunks (n->decl);
> 
> Hopefully we can move thunks into same body aliases too incrementally.
> 

Well, thunks would need some extra info, but yes.

> > @@ -1910,7 +1923,22 @@ cgraph_materialize_all_clones (void)
> >  	      {
> >  		gimple new_stmt;
> >  		gimple_stmt_iterator gsi;
> > -		
> > +
> > +		if (e->callee->same_body)
> > +		  {
> > +		    struct cgraph_node *alias;
> > +
> > +		    for (alias = e->callee->same_body;
> > +			 alias;
> > +			 alias = alias->next)
> > +		      if (decl == alias->decl)
> > +			break;
> > +		    /* Don't update call from same body alias to the real
> > +		       function.  */
> > +		    if (alias)
> > +		      continue;
> 
> I don't get this.  You have functions a,b,c,d in same body group and you
> make cgraph so all calleers ends going to a, then you constant propagate
> and discover that first argument of a is always 1 but a/b/c/d are also
> exported from unit, so you decide to clone.
> In this case you want to update all the calls no matter if they go to
> a,b,c, or d, since they are all having first argument of 1.

This was to cure something I've noticed when eyeballing the output.
Which symbol is the real one and what are the aliases is quite arbitrary
choice, and without this hunk cgraph_materialize_all_clones was rewriting
all the calls to the real fn.  That's IMNSHO undesirable, if some function
calls _ZN1SD1Ev, we shouldn't change that call to _ZN1SD2Ev just because
we made in this CU an arbitrary choice to emit _ZN1SD2Ev as the real
function and _ZN1SD1Ev as an alias to it.  The call relocation will be
visible in the object file (unlike what was the real function and what were
the aliases).  While if in one CU we decided that both destructors are
identical they'd better be identical in all other CUs, shared libraries,
etc., it is still IMHO a good idea not to rewrite the callers needlessly.
People might be surprised why the code calls say base dtor when it is
supposed to call complete dtor, etc.

> Also can it happen for a/b/c/d to have different visibilities? (ie. some
> externally visible, others not?)

Currently they must have the same visibilities, same TREE_PUBLIC, etc.

	Jakub