Re: Fix code size estimates wrt variadic functions

[Richard Guenther <rguenther at suse dot de>]

On Tue, 1 Jun 2010, Jan Hubicka wrote:

> Hi,
> another common bogus inlines are trim_filename and rtl_checking_flag_failed.
> They are called from error checking code that inliner should refuse to inline
> because calls should be hot because of noreturn flag.
> 
> The reason for this is more stubble. The checking things takes usually many
> arguments and pass them to internal_error.
> When counting call cost we look into declaration arguments and thus we count
> variadic function calls as cheap.  This leads inliner to think that inlining
> those wrappers is going to reduce code size.
> 
> Fixed as follows.
> 
> Bootstraped/regtested x86_64-linux, OK?
> 
> Honza
> 
> 	* tree-inline.c (estimate_num_insns): For stdarg functions look
> 	into call statement to count cost of argument passing.
> Index: tree-inline.c
> ===================================================================
> --- tree-inline.c	(revision 160079)
> +++ tree-inline.c	(working copy)
> @@ -3367,6 +3367,7 @@
>  	tree decl = gimple_call_fndecl (stmt);
>  	tree addr = gimple_call_fn (stmt);
>  	tree funtype = TREE_TYPE (addr);
> +	bool stdarg = false;
>  
>  	if (POINTER_TYPE_P (funtype))
>  	  funtype = TREE_TYPE (funtype);
> @@ -3475,17 +3476,26 @@
>  
>  	if (!VOID_TYPE_P (TREE_TYPE (funtype)))
>  	  cost += estimate_move_cost (TREE_TYPE (funtype));
> +
> +	if (funtype)
> +	  stdarg = stdarg_p (funtype);
> +
>  	/* Our cost must be kept in sync with
>  	   cgraph_estimate_size_after_inlining that does use function
> -	   declaration to figure out the arguments.  */
> -	if (decl && DECL_ARGUMENTS (decl))
> +	   declaration to figure out the arguments.
> +
> +	   For functions taking variable list of arguments we must
> +	   look into call statement intself.  This is safe because
> +	   we will get only higher costs and in most cases we will
> +	   not inline these anyway.  */
> +	if (decl && DECL_ARGUMENTS (decl) && !stdarg)
>  	  {
>  	    tree arg;
>  	    for (arg = DECL_ARGUMENTS (decl); arg; arg = TREE_CHAIN (arg))
>  	      if (!VOID_TYPE_P (TREE_TYPE (arg)))
>  	        cost += estimate_move_cost (TREE_TYPE (arg));
>  	  }
> -	else if (funtype && prototype_p (funtype))
> +	else if (funtype && prototype_p (funtype) && !stdarg)
>  	  {
>  	    tree t;
>  	    for (t = TYPE_ARG_TYPES (funtype); t && t != void_list_node;

I think this whole chain of different ways to look at argument cost
should go away by simply always looking at the call stmt arguments
(where the !VOID_TYPE_P (TREE_TYPE (arg)) check is useless as well).

Can you rework the patch that way?

Ok with this change.

Thanks,
Richard.