Re: [PATCH] Propagate nonfreeing_call_p using ipa-pure-const

[Jan Hubicka <hubicka@ucw.cz>]

> On Wed, Nov 12, 2014 at 02:49:30PM +0400, Maxim Ostapenko wrote:
> > We used this code for IPA propagation of nonfreeing_call_p. It implemented
> > with a separate pass, but it probably could be propagated in some existing
> > one. This analysis doesn't seem to be costly thought, we didn't see any
> > significant slowdown compiling big files.
> 
> Here it is rewritten using ipa-pure-const which is where Richard/Honza
> suggested it should be done in.
> 
> I wonder if the nonfreeing_call_p function shouldn't be moved elsewhere
> though (suggestion where), so that gimple.c doesn't need the cgraph
> includes.
> 
> In any case, bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2014-11-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* ipa-pure-const.c (struct funct_state_d): Add can_free field.
> 	(varying_state): Add true for can_free.
> 	(check_call): For builtin or internal !nonfreeing_call_p set
> 	local->can_free.
> 	(check_stmt): For asm volatile and asm with "memory" set
> 	local->can_free.
> 	(analyze_function): Clear local->can_free initially, continue
> 	calling check_stmt until all flags are computed, dump can_free
> 	flag.
> 	(pure_const_write_summary): Write can_free flag.
> 	(pure_const_read_summary): Read it back.
> 	(propagate_can_free): New function.
> 	(pass_ipa_pure_const::execute): Call it.
> 	* cgraph.h (cgraph_node): Add nonfreeing_fn member.
> 	* gimple.c: Include ipa-ref.h, lto-streamer.h and cgraph.h.
> 	(nonfreeing_call_p): Return cgraph nonfreeing_fn flag if set.
> 	* cgraph.c (cgraph_node::dump): Dump nonfreeing_fn flag.
> 	* lto-cgraph.c (lto_output_node): Write nonfreeing_fn flag.
> 	(input_overwrite_node): Read it back.
> 
> --- gcc/ipa-pure-const.c.jj	2014-11-12 18:32:56.351139726 +0100
> +++ gcc/ipa-pure-const.c	2014-11-12 21:11:08.574354600 +0100
> @@ -112,11 +112,15 @@ struct funct_state_d
>    bool looping;
>  
>    bool can_throw;
> +
> +  /* If function can call free, munmap or otherwise make previously
> +     non-trapping memory accesses trapping.  */
> +  bool can_free;
>  };
>  
>  /* State used when we know nothing about function.  */
>  static struct funct_state_d varying_state
> -   = { IPA_NEITHER, IPA_NEITHER, true, true, true };
> +   = { IPA_NEITHER, IPA_NEITHER, true, true, true, true };
>  
>  
>  typedef struct funct_state_d * funct_state;
> @@ -559,6 +563,10 @@ check_call (funct_state local, gimple ca
>        enum pure_const_state_e call_state;
>        bool call_looping;
>  
> +      if (gimple_call_builtin_p (call, BUILT_IN_NORMAL)
> +	  && !nonfreeing_call_p (call))
> +	local->can_free = true;
> +
>        if (special_builtin_state (&call_state, &call_looping, callee_t))
>  	{
>  	  worse_state (&local->pure_const_state, &local->looping,
> @@ -589,6 +597,8 @@ check_call (funct_state local, gimple ca
>  	    break;
>  	  }
>      }
> +  else if (gimple_call_internal_p (call) && !nonfreeing_call_p (call))
> +    local->can_free = true;

Actually I think you want to do this for can_throw, too.
We probably do not have throwing internal calls, but it is better to be safe.
> +/* Produce transitive closure over the callgraph and compute can_free
> +   attributes.  */
> +
> +static void
> +propagate_can_free (void)
> +{
> +  struct cgraph_node *node;
> +  struct cgraph_node *w;
> +  struct cgraph_node **order
> +    = XCNEWVEC (struct cgraph_node *, symtab->cgraph_count);
> +  int order_pos;
> +  int i;
> +  struct ipa_dfs_info *w_info;
> +
> +  order_pos = ipa_reduced_postorder (order, true, false, NULL);
> +  if (dump_file)
> +    {
> +      cgraph_node::dump_cgraph (dump_file);
> +      ipa_print_order (dump_file, "reduced", order, order_pos);
> +    }

The propagation seems fine, but I wonder if we won't get better memory locality doing this
during the propagation of pure/const?

nothrow flag goes in separate loop because knowledge of nothrow helps pure/const to work better.
Also one can ignore call edges that are !can_thros_externally to get fewer cycles, but apparently
this got never implemented.
>  	   && gimple_call_flags (call) & ECF_LEAF)
>      return true;
>  
> -  return false;
> +  tree fndecl = gimple_call_fndecl (call);
> +  if (!fndecl)
> +    return false;
> +  struct cgraph_node *n = cgraph_node::get (fndecl);

You want to walk aliases,

  cgraph_node::get (fndecl)->function_symbol (&availability)

> +  if (!n || n->get_availability () <= AVAIL_INTERPOSABLE)

and use availability here.

OK with this change.

Honza