[PATCH 2/2, PR 63814] Do not re-create expanded artificial thunks

Jan Hubicka hubicka@ucw.cz
Thu Dec 4 17:48:00 GMT 2014


> 
> I did not really like it either (although I must say that in general I
> really dislike the need to call expand_thunk but telling it to not

Yeah, I am thinking about simply teaching inliner to handle them.  Given the issues
with extra copies for values passed by reference it may be better solution.

> really expand anything if possible).  So here is another approach,
> which works by delaying expansion of thunks but only after all
> redirections within create_clone are done.  This has the advantage
> that the new method expand_all_artificial_thunks is not exposed to
> every (indirect) user of cgraph_node::create_clone (as opposed to
> requiring them all to call that function at some point after making
> any redirections at all).  However, it also has the disadvantage that
> in the unlikeliest of circumstances (strongly connected components in
> IPA-CP value dependencies connected by thunks and these thunks ceasing
> to be assembly thunks in the course of cloning at the same time),
> IPA-CP might still create extra gimple-expanded thunks.

This is because you expand the thunks during creating and lose track of them?
> 
> The alternative not suffering from this problem would be either
> exposing expand_all_artificial_thunks to all users and requiring them
> to call it, with IPA-CP doing it for all created clones at the very
> end of propagation, or teaching the infrastructure and pass manager
> about this and then expanding them at some convenient time
> (before/after inlining?).  However, the first option seems to be too
> ugly for very little benefit (but I am willing to implement that) and
> the second does not seem to be stage3 material.
> 
> What do you think?  Bootstrapped and tested on x86_64-linux.

Hmm, I think thunks in SCC regions are rather rare and small.  Lets go with this
and try to look whether we can't get thunks more regular in inliner.
> 
> Martin
> 
> 
> 2014-12-02  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraph.h (cgraph_node): New method expand_all_artificial_thunks.
> 	(cgraph_edge): New method redirect_callee_duplicating_thunks.
> 	* cgraphclones.c (duplicate_thunk_for_node): Donot expand newly
> 	created thunks.
> 	(redirect_edge_duplicating_thunks): Turned into edge method
> 	redirect_callee_duplicating_thunks.
> 	(cgraph_node::expand_all_artificial_thunks): New method.
> 	(create_clone): Call expand_all_artificial_thunks.
> 	* ipa-cp.c (perhaps_add_new_callers): Call
> 	redirect_callee_duplicating_thunks instead of redirect_callee.
> 	Also call expand_all_artificial_thunks.

OK
Honza
> 
> Index: src/gcc/cgraph.h
> ===================================================================
> --- src.orig/gcc/cgraph.h
> +++ src/gcc/cgraph.h
> @@ -908,6 +908,10 @@ public:
>       thunks that are not lowered.  */
>    bool expand_thunk (bool output_asm_thunks, bool force_gimple_thunk);
>  
> +  /*  Call expand_thunk on all callers that are thunks and if analyze those
> +      nodes that were expanded.  */
> +  void expand_all_artificial_thunks ();
> +
>    /* Assemble thunks and aliases associated to node.  */
>    void assemble_thunks_and_aliases (void);
>  
> @@ -1477,6 +1481,12 @@ struct GTY((chain_next ("%h.next_caller"
>       call expression.  */
>    void redirect_callee (cgraph_node *n);
>  
> +  /* If the edge does not lead to a thunk, simply redirect it to N.  Otherwise
> +     create one or more equivalent thunks for N and redirect E to the first in
> +     the chain.  Note that it is then necessary to call
> +     n->expand_all_artificial_thunks once all callers are redirected.  */
> +  void redirect_callee_duplicating_thunks (cgraph_node *n);
> +
>    /* Make an indirect edge with an unknown callee an ordinary edge leading to
>       CALLEE.  DELTA is an integer constant that is to be added to the this
>       pointer (first parameter) to compensate for skipping
> Index: src/gcc/cgraphclones.c
> ===================================================================
> --- src.orig/gcc/cgraphclones.c
> +++ src/gcc/cgraphclones.c
> @@ -370,28 +370,47 @@ duplicate_thunk_for_node (cgraph_node *t
>  						  CGRAPH_FREQ_BASE);
>    e->call_stmt_cannot_inline_p = true;
>    symtab->call_edge_duplication_hooks (thunk->callees, e);
> -  if (new_thunk->expand_thunk (false, false))
> -    {
> -      new_thunk->thunk.thunk_p = false;
> -      new_thunk->analyze ();
> -    }
> -
>    symtab->call_cgraph_duplication_hooks (thunk, new_thunk);
>    return new_thunk;
>  }
>  
>  /* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
>     one or more equivalent thunks for N and redirect E to the first in the
> -   chain.  */
> +   chain.  Note that it is then necessary to call
> +   n->expand_all_artificial_thunks once all callers are redirected.  */
>  
>  void
> -redirect_edge_duplicating_thunks (cgraph_edge *e, cgraph_node *n)
> +cgraph_edge::redirect_callee_duplicating_thunks (cgraph_node *n)
>  {
> -  cgraph_node *orig_to = e->callee->ultimate_alias_target ();
> +  cgraph_node *orig_to = callee->ultimate_alias_target ();
>    if (orig_to->thunk.thunk_p)
>      n = duplicate_thunk_for_node (orig_to, n);
>  
> -  e->redirect_callee (n);
> +  redirect_callee (n);
> +}
> +
> +/* Call expand_thunk on all callers that are thunks and if analyze those nodes
> +   that were expanded.  */
> +
> +void
> +cgraph_node::expand_all_artificial_thunks ()
> +{
> +  cgraph_edge *e;
> +  for (e = callers; e;)
> +    if (e->caller->thunk.thunk_p)
> +      {
> +	cgraph_node *thunk = e->caller;
> +
> +	e = e->next_caller;
> +	if (thunk->expand_thunk (false, false))
> +	  {
> +	    thunk->thunk.thunk_p = false;
> +	    thunk->analyze ();
> +	  }
> +	thunk->expand_all_artificial_thunks ();
> +      }
> +    else
> +      e = e->next_caller;
>  }
>  
>  /* Create node representing clone of N executed COUNT times.  Decrease
> @@ -483,8 +502,9 @@ cgraph_node::create_clone (tree decl, gc
>        if (!e->callee
>  	  || DECL_BUILT_IN_CLASS (e->callee->decl) != BUILT_IN_NORMAL
>  	  || DECL_FUNCTION_CODE (e->callee->decl) != BUILT_IN_UNREACHABLE)
> -        redirect_edge_duplicating_thunks (e, new_node);
> +        e->redirect_callee_duplicating_thunks (new_node);
>      }
> +  new_node->expand_all_artificial_thunks ();
>  
>    for (e = callees;e; e=e->next_callee)
>      e->clone (new_node, e->call_stmt, e->lto_stmt_uid, count_scale,
> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -3806,7 +3806,8 @@ perhaps_add_new_callers (cgraph_node *no
>  			 xstrdup (val->spec_node->name ()),
>  			 val->spec_node->order);
>  
> -	      cs->redirect_callee (val->spec_node);
> +	      cs->redirect_callee_duplicating_thunks (val->spec_node);
> +	      val->spec_node->expand_all_artificial_thunks ();
>  	      redirected_sum += cs->count;
>  	    }
>  	  cs = get_next_cgraph_edge_clone (cs);



More information about the Gcc-patches mailing list