This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Clean up early inliner


On Fri, 9 Apr 2010, Martin Jambor wrote:

> Hi,
> 
> On Wed, Apr 07, 2010 at 11:31:03AM +0200, Richard Guenther wrote:
> > 
> > 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?
> > 
> > Thanks,
> > Richard.
> > 
> > 2010-01-29  Richard Guenther  <rguenther@suse.de>
> > 
> > 	* ipa.c (cgraph_postorder): Adjust postorder to guarantee
> > 	single-iteration always-inline inlining.
> > 	* ipa-inline.c (cgraph_mark_inline): Do not return anything.
> > 	(cgraph_decide_inlining): Do not handle always-inline
> > 	specially.
> > 	(try_inline): Remove always-inline cycle detection special case.
> > 	Do not recurse on always-inlines.
> > 	(cgraph_early_inlining): Do not iterate if not optimizing.
> > 	(cgraph_gate_early_inlining): remove.
> > 	(pass_early_inline): Run unconditionally.
> > 	(gate_cgraph_decide_inlining): New function.
> > 	(pass_ipa_inline): Use it.  Do not run the IPA inliner if
> > 	not inlining or optimizing.
> > 
> > 	* gcc.dg/torture/inline-2.c: New testcase.
> > 
> 
> ...
> 
> > *************** 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)
> > + {
> > +   struct cgraph_edge *e;
> > +   void *old_mode;
> > + 
> > +   old_mode = node->aux;
> 
> I believe that the code below (the test of callee_node) assumes that
> all unprocessed nodes have node->aux NULL and after having briefly
> glanced at the call sites I tend to think it is correct.  Therefore I
> would consider asserting that and setting it back to NULL at the end
> of the function.
> 
> > + 
> > +   node->aux = (void *)(size_t) INLINE_ALL;
> > + 
> > +   for (e = node->callees; e; e = e->next_callee)
> > +     {
> > +       struct cgraph_node *callee = e->callee;
> 
> I find it quite confusing that throughout this cycle we refer to the
> same node as both callee and e->callee in a rather random fashion.
> Wouldn't we be better off without this variable?
> 
> > +       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;
> 
> Aren't these assignments to callee->aux redundant?  It seems that
> cgraph_flatten does the very same thing.
> 
> > + 	  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;
> > + 	}
> > + 
> > +       if (dump_file)
> > + 	fprintf (dump_file, " Inlining %s into %s.\n",
> > + 		 cgraph_node_name (e->callee),
> > + 		 cgraph_node_name (e->caller));
> > +       if (e->inline_failed)
> > + 	{
> > + 	  cgraph_mark_inline (e);
> > + 
> > + 	  /* Flattening needs to be done recursively.  */
> > + 	  callee->aux = (void *)(size_t) INLINE_ALL;
> > + 	  cgraph_flatten (e->callee);
> > + 	  callee->aux = callee_mode;
> 
> Likewise.

Indeed.  I copy & pasted from the original code and did some manual
inlining.  Thanks for spotting these possible cleanups, I've applied
them (and some more).

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]