[PATCH] Clean up early inliner

Jan Hubicka hubicka@ucw.cz
Wed Apr 7 09:50:00 GMT 2010


> 
> This cleans up the early inliner, simplifying it and making it the
> one that resolves all always-inline inlines.  It also should remove
> the need to run the IPA inliner when not optimizing or not inlining.
> 
> The single early inlining cleanup that is left is the interaction
> with profiling.
> 
> Boostrap and regtest running on x86_64-unknown-linux-gnu, ok for
> trunk?

Are we going to set some politics concerning always inline and indirect calls?
I.e. early inliner can handle with some cases of indirect calls. Code you
remove in IPA inliner is able to handle always inline functions having callbacks
that are always inline.  I am not convinced we need to make any promises here,
but we are losing some "feature".

I am leaving soon, so give me some time to thing about the cyclic graphs.
I am not completely happy about the fact that topological sorter now ignore
all callers of always inline functions, at least that it does so unconditionally.
It will lead to some pesimization of those.
(i.e. if you have A that calls always inline B that calls C, topological sorter
will no longer make C early optimized before A that will result in early inliner
not inlining the whole chaing A->B->C.  This is probably something you don't want
when you care about performance enough to play with always inlines)

> *************** cgraph_decide_inlining_of_small_function
> *** 1127,1132 ****
> --- 1124,1211 ----
>     BITMAP_FREE (updated_nodes);
>   }
>   
> + /* Flatten NODE from the IPA inliner.  */
> + 
> + static void
> + cgraph_flatten (struct cgraph_node *node)

When the flattening code is broken out, I guess early inliner could do the job too.
That will result in early optimizations having more chance to further simplify the
code.  But I don't care much (how many uses this found except for tramp3d?)

> + {
> +   struct cgraph_edge *e;
> +   void *old_mode;
> + 
> +   old_mode = node->aux;
> + 
> +   node->aux = (void *)(size_t) INLINE_ALL;
> + 
> +   for (e = node->callees; e; e = e->next_callee)
> +     {
> +       struct cgraph_node *callee = e->callee;
> +       void *callee_mode = callee->aux;
> + 
> +       if (e->call_stmt_cannot_inline_p)
> + 	continue;
> + 
> +       /* We've hit cycle?  It is time to give up.  */
> +       if (callee_mode)
> + 	{
> + 	  if (dump_file)
> + 	    fprintf (dump_file,
> + 		     "Not inlining %s into %s to avoid cycle.\n",
> + 		     cgraph_node_name (callee),
> + 		     cgraph_node_name (e->caller));
> + 	  e->inline_failed = (e->callee->local.disregard_inline_limits
> + 			      ? CIF_RECURSIVE_INLINING : CIF_UNSPECIFIED);
> + 	  continue;
> + 	}
> + 
> +       /* When the edge is already inlined, we just need to recurse into
> + 	 it in order to fully flatten the leaves.  */
> +       if (!e->inline_failed)
> + 	{
> + 	  callee->aux = (void *)(size_t) INLINE_ALL;
> + 	  cgraph_flatten (e->callee);
> + 	  callee->aux = callee_mode;
> + 	  continue;
> + 	}
> + 
> +       if (cgraph_recursive_inlining_p (node, e->callee, &e->inline_failed))
> + 	{
> + 	  if (dump_file)
> + 	    fprintf (dump_file, "Not inlining: recursive call.\n");
> + 	  continue;
> + 	}
> +       if (!tree_can_inline_p (e))
> + 	{
> + 	  if (dump_file)
> + 	    fprintf (dump_file, "Not inlining: %s",
> + 		     cgraph_inline_failed_string (e->inline_failed));
> + 	  continue;
> + 	}
> +       if (!e->callee->analyzed)
> + 	{
> + 	  if (dump_file)
> + 	    fprintf (dump_file,
> + 		     "Not inlining: Function body no longer available.\n");
> + 	  continue;
> + 	}

Funcition bodies should not disappear anymore.   I guess this is first test
that checks if function body is around, so I guess you should put it first for
effectivity  and change working to "Function body not available".

> + /* When to run IPA inlining.  Inlining of always-inline functions
> +    happens during early inlining.  */
> + 
> + static bool
> + gate_cgraph_decide_inlining (void)
> + {
> +   /* We'd like to skip this if not optimizing or not inlining as
> +      all always-inline functions have been processed by early
> +      inlining already.  But this breaks EH with C++ somehow with
> + 
> +      g++.dg/torture/pr31863.C: In destructor 'Serializer<unsigned int, Loki::Typelist<ClassSpec<unsigned int, A040, 40u>, Loki::NullType> >::~Serializer()':
> +      g++.dg/torture/pr31863.C:231:7: error: statement marked for throw, but doesn't
> +      Serializer<unsigned int, ClassSpec<unsigned int, A040, 40u> >::~Serializer (this.18352_8, D.118411_7);
> + 
> +      so leave it on unconditionally for now.  */

This is because inliner calls the fixup_cfg pass that is needed for other
reasons too.  I guess we can just make new IPA pass that will have only
transform method that will call the fixups scheduled after IPA_INLINER.
(that is probablyy cleaner too)

Thanks for working on this :) 

Honza



More information about the Gcc-patches mailing list