[PATCH] Make -Os inlining less magic

[Richard Guenther]

On Wed, 2 May 2007, Jan Hubicka wrote:

> > 
> > This removes some magic constnats we use for inlining for -Os by just
> > forcing the inliner to never grow what is inlined into.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu - I'm a bit nervous
> > about the not-needed called once function change -- we cannot easily
> > tell that apart from the regular case in cgraph_check_inline_limits().
> 
> Actually I would say that cgraph_check_inline_limits should be follwed
> here too - ie the only check in effect should be a check verifying that
> caller body is not growing too much.  This is to prevent GCC from
> exhibiting quadratic (or worse) behaviour that we don't want even on -Os

Sure, but cgraph_check_inline_limits as patched will reject to inline
functions called once that grow the caller body at all...

Basically, in this case we want to ignore that we are optimizing for size.
So there are two possibilities - copy what is needed from 
cgraph_check_inline_limits into the caller or unset optimize_size around
the call.  (or add another param to cgraph_check_inline_limits)

Thoughts?

Richard.

> 
> Otherwise it is OK.
> Thanks,
> Honza
> > 
> > Ok?
> > 
> > Thanks,
> > Richard.
> > 
> > 2007-05-01  Richard Guenther  <rguenther@suse.de>
> > 
> > 	* opts.c (decode_options): Do not fiddle with inlining
> > 	parameters in case of optimizing for size.
> > 	* ipa-inline.c (cgraph_check_inline_limits): If optimizing
> > 	for size make sure we do not grow the unit-size by inlining.
> > 	(cgraph_decide_recursive_inlining): Likewise.
> > 	(compute_max_insns): Likewise.
> > 	(cgraph_decide_inlining): Always inline unneeded functions
> > 	called once if optimizing for size.
> > 
> > Index: opts.c
> > ===================================================================
> > *** opts.c.orig	2007-04-21 14:07:46.000000000 +0200
> > --- opts.c	2007-05-01 11:36:34.000000000 +0200
> > *************** decode_options (unsigned int argc, const
> > *** 796,804 ****
> >   
> >     if (optimize_size)
> >       {
> > !       /* Inlining of very small functions usually reduces total size.  */
> > !       set_param_value ("max-inline-insns-single", 5);
> > !       set_param_value ("max-inline-insns-auto", 5);
> >         flag_inline_functions = 1;
> >   
> >         /* We want to crossjump as much as possible.  */
> > --- 796,803 ----
> >   
> >     if (optimize_size)
> >       {
> > !       /* Inlining of functions reducing size is a good idea regardless
> > ! 	 of them being declared inline.  */
> >         flag_inline_functions = 1;
> >   
> >         /* We want to crossjump as much as possible.  */
> > Index: ipa-inline.c
> > ===================================================================
> > *** ipa-inline.c.orig	2007-03-14 22:29:31.000000000 +0100
> > --- ipa-inline.c	2007-05-01 11:59:16.000000000 +0200
> > *************** cgraph_check_inline_limits (struct cgrap
> > *** 371,377 ****
> >     /* Check the size after inlining against the function limits.  But allow
> >        the function to shrink if it went over the limits by forced inlining.  */
> >     newsize = cgraph_estimate_size_after_inlining (times, to, what);
> > !   if (newsize >= to->global.insns
> >         && newsize > PARAM_VALUE (PARAM_LARGE_FUNCTION_INSNS)
> >         && newsize > limit)
> >       {
> > --- 371,384 ----
> >     /* Check the size after inlining against the function limits.  But allow
> >        the function to shrink if it went over the limits by forced inlining.  */
> >     newsize = cgraph_estimate_size_after_inlining (times, to, what);
> > !   if (optimize_size
> > !       && newsize > to->global.insns)
> > !     {
> > !       if (reason)
> > ! 	*reason = N_("optimizing for size");
> > !       return false;
> > !     }
> > !   if (newsize > to->global.insns
> >         && newsize > PARAM_VALUE (PARAM_LARGE_FUNCTION_INSNS)
> >         && newsize > limit)
> >       {
> > *************** cgraph_decide_recursive_inlining (struct
> > *** 667,672 ****
> > --- 674,680 ----
> >     struct cgraph_node *master_clone, *next;
> >     int depth = 0;
> >     int n = 0;
> > +   int newsize;
> >   
> >     if (DECL_DECLARED_INLINE_P (node->decl))
> >       {
> > *************** cgraph_decide_recursive_inlining (struct
> > *** 675,682 ****
> >       }
> >   
> >     /* Make sure that function is small enough to be considered for inlining.  */
> >     if (!max_depth
> > !       || cgraph_estimate_size_after_inlining (1, node, node)  >= limit)
> >       return false;
> >     heap = fibheap_new ();
> >     lookup_recursive_calls (node, node, heap);
> > --- 683,692 ----
> >       }
> >   
> >     /* Make sure that function is small enough to be considered for inlining.  */
> > +   newsize = cgraph_estimate_size_after_inlining (1, node, node);
> >     if (!max_depth
> > !       || newsize >= limit
> > !       || (optimize_size && newsize > node->global.insns))
> >       return false;
> >     heap = fibheap_new ();
> >     lookup_recursive_calls (node, node, heap);
> > *************** cgraph_set_inline_failed (struct cgraph_
> > *** 795,804 ****
> > --- 805,819 ----
> >   
> >   /* Given whole compilation unit estimate of INSNS, compute how large we can
> >      allow the unit to grow.  */
> > + 
> >   static int
> >   compute_max_insns (int insns)
> >   {
> >     int max_insns = insns;
> > + 
> > +   if (optimize_size)
> > +     return insns;
> > + 
> >     if (max_insns < PARAM_VALUE (PARAM_LARGE_UNIT_INSNS))
> >       max_insns = PARAM_VALUE (PARAM_LARGE_UNIT_INSNS);
> >   
> > *************** cgraph_decide_inlining (void)
> > *** 1129,1136 ****
> >   
> >   	      old_insns = overall_insns;
> >   
> > ! 	      if (cgraph_check_inline_limits (node->callers->caller, node,
> > ! 					      NULL, false))
> >   		{
> >   		  cgraph_mark_inline (node->callers);
> >   		  if (dump_file)
> > --- 1144,1152 ----
> >   
> >   	      old_insns = overall_insns;
> >   
> > ! 	      if (optimize_size
> > ! 		  || cgraph_check_inline_limits (node->callers->caller, node,
> > ! 						 NULL, true))
> >   		{
> >   		  cgraph_mark_inline (node->callers);
> >   		  if (dump_file)
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex