Re: loop-ch tweek

[Richard Biener <rguenther at suse dot de>]

On Sun, 5 Jun 2016, Jan Hubicka wrote:

> Hi,
> both loop-ch and loop-ivcanon want to trottle down the heuristics on paths
> containing call. Testing for presence of GIMPLE_CALL is wrong for internal
> call and cheap builtins that are expanded inline.
> 
> Bootstrapped/regtested x86_64-linux, OK?

First of all the name is bad - I'd say gimple_inexpensive_call_p ()
is better.  More comments below.

> Honza
> 
> 	* gimple.c: Include builtins.h.
> 	(gimple_real_call_p): New function.
> 	* gimple.h (gimple_real_call_p): Declare.
> 	* tree-ssa-loop-ch.c (should_duplicate_loop_header_p): Use it.
> 	* tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Likewise.
> Index: gimple.c
> ===================================================================
> --- gimple.c	(revision 237101)
> +++ gimple.c	(working copy)
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.
>  #include "gimple-walk.h"
>  #include "gimplify.h"
>  #include "target.h"
> +#include "builtins.h"
>  
>  
>  /* All the tuples have their operand vector (if present) at the very bottom
> @@ -3018,3 +3019,20 @@ maybe_remove_unused_call_args (struct fu
>        update_stmt_fn (fn, stmt);
>      }
>  }
> +
> +/* Return true if STMT will likely expand to real call statment.  */
> +
> +bool
> +gimple_real_call_p (gimple *stmt)
> +{
> +  if (gimple_code (stmt) != GIMPLE_CALL)
> +    return false;
> +  if (gimple_call_internal_p (stmt))
> +    return false;
> +  tree decl = gimple_call_fndecl (stmt);
> +  if (decl && DECL_IS_BUILTIN (decl)
> +      && (is_simple_builtin (decl)
> +	  || is_inexpensive_builtin (decl)))

is_inexpensive_builtin includes is_simple_builtin handling and also
tests decl && DECL_IS_BUILTIN properly already.

> +    return false;
> +  return true;
> +}
> Index: gimple.h
> ===================================================================
> --- gimple.h	(revision 237101)
> +++ gimple.h	(working copy)
> @@ -1525,6 +1525,7 @@ extern void preprocess_case_label_vec_fo
>  extern void gimple_seq_set_location (gimple_seq, location_t);
>  extern void gimple_seq_discard (gimple_seq);
>  extern void maybe_remove_unused_call_args (struct function *, gimple *);
> +extern bool gimple_real_call_p (gimple *);
>  
>  /* Formal (expression) temporary table handling: multiple occurrences of
>     the same scalar expression are evaluated into the same temporary.  */
> Index: tree-ssa-loop-ch.c
> ===================================================================
> --- tree-ssa-loop-ch.c	(revision 237101)
> +++ tree-ssa-loop-ch.c	(working copy)
> @@ -118,7 +118,7 @@ should_duplicate_loop_header_p (basic_bl
>        if (is_gimple_debug (last))
>  	continue;
>  
> -      if (is_gimple_call (last))
> +      if (gimple_real_call_p (last))
>  	{
>  	  if (dump_file && (dump_flags & TDF_DETAILS))
>  	    fprintf (dump_file,
> Index: tree-ssa-loop-ivcanon.c
> ===================================================================
> --- tree-ssa-loop-ivcanon.c	(revision 237101)
> +++ tree-ssa-loop-ivcanon.c	(working copy)
> @@ -339,15 +339,11 @@ tree_estimate_loop_size (struct loop *lo
>        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>  	{
>  	  gimple *stmt = gsi_stmt (gsi);
> -	  if (gimple_code (stmt) == GIMPLE_CALL)
> +	  if (gimple_code (stmt) == GIMPLE_CALL
> +	      && gimple_real_call_p (stmt))

gimple_code (stmt) == GIMPLE_CALL is redundant then.  I'd prefer to
make gimple_inexpensive_call_p take a gcall * argument and do the
test at the callers though.

>  	    {
>  	      int flags = gimple_call_flags (stmt);
> -	      tree decl = gimple_call_fndecl (stmt);
> -
> -	      if (decl && DECL_IS_BUILTIN (decl)
> -		  && is_inexpensive_builtin (decl))
> -		;
> -	      else if (flags & (ECF_PURE | ECF_CONST))
> +	      if (flags & (ECF_PURE | ECF_CONST))
>  		size->num_pure_calls_on_hot_path++;
>  	      else
>  		size->num_non_pure_calls_on_hot_path++;
> 
>