Re: [PATCH] Make -Os inlining less magic

[Jan Hubicka <hubicka at ucw dot cz>]

> 
> 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

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)