Enable ipa-sra for functions with fnspec attribute

  Hi,
this patch enables some ipa-sra on fortran by allowing signature changes on functions
with "fn spec" attribute when ipa-modref is enabled.  This is possible since ipa-modref
knows how to preserve things we trace in fnspec and fnspec generated by fortran forntend
are quite simple and can be analysed automatically now.  To be sure I will also add
code that merge fnspec to parameters.

This unfortunately hits bug in ipa-param-manipulation when we remove parameter
that specifies size of variable length parameter. For this reason I added a hack
that prevent signature changes on such functions and will handle it incrementally.

I tried creating C testcase but it is blocked by another problem that we punt ipa-sra
on access attribute.  This is optimization regression we ought to fix so I filled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103223.

As a followup I will add code classifying the type attributes (we have just few) and 
get stats on access attribute.

Martin, can you please check that the code detecting signature changes is correct
and can't be done more easily?

Bootstrapped/regtested x86_64-linux, comitted.
Honza

gcc/ChangeLog:

	* ipa-fnsummary.c (compute_fn_summary): Do not give up on signature
	changes on "fn spec" attribute; give up on varadic types.
	* ipa-param-manipulation.c: Include attribs.h.
	(build_adjusted_function_type): New parameter ARG_MODIFIED; if it is
	true remove "fn spec" attribute.
	(ipa_param_adjustments::build_new_function_type): Update.
	(ipa_param_body_adjustments::modify_formal_parameters): update.
	* ipa-sra.c: Include attribs.h.
	(ipa_sra_preliminary_function_checks): Do not check for TYPE_ATTRIBUTES.
  

Hi,

On Sat, Nov 13 2021, Jan Hubicka via Gcc-patches wrote:
> Hi,
> this patch enables some ipa-sra on fortran by allowing signature changes on functions
> with "fn spec" attribute when ipa-modref is enabled.  This is possible since ipa-modref
> knows how to preserve things we trace in fnspec and fnspec generated by fortran forntend
> are quite simple and can be analysed automatically now.  To be sure I will also add
> code that merge fnspec to parameters.
>
> This unfortunately hits bug in ipa-param-manipulation when we remove parameter
> that specifies size of variable length parameter. For this reason I added a hack
> that prevent signature changes on such functions and will handle it incrementally.
>
> I tried creating C testcase but it is blocked by another problem that we punt ipa-sra
> on access attribute.  This is optimization regression we ought to fix so I filled
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103223.
>
> As a followup I will add code classifying the type attributes (we have just few) and 
> get stats on access attribute.
>
> Martin, can you please check that the code detecting signature changes is correct

I think both ways of detecting it in the patch are correct, in the sense
that if the clone in question does not modify parameters but is a clone
of another clone which does, the methods would still consider it a
changing clone (to do otherwise they would need to check
prev_clone_index and not base_index).  But I don't think the difference
matters here.  It might if we start updating the strings of the
attributes where position is important.

> ...and can't be done more easily?

in the case of ipa_param_body_adjustments you could introduce another
flag of the class and set it whenever any of the elements in local
vector kept in common_initialization() turned out not to be true.

Martin


>
> Bootstrapped/regtested x86_64-linux, comitted.
> Honza
>
> gcc/ChangeLog:
>
> 	* ipa-fnsummary.c (compute_fn_summary): Do not give up on signature
> 	changes on "fn spec" attribute; give up on varadic types.
> 	* ipa-param-manipulation.c: Include attribs.h.
> 	(build_adjusted_function_type): New parameter ARG_MODIFIED; if it is
> 	true remove "fn spec" attribute.
> 	(ipa_param_adjustments::build_new_function_type): Update.
> 	(ipa_param_body_adjustments::modify_formal_parameters): update.
> 	* ipa-sra.c: Include attribs.h.
> 	(ipa_sra_preliminary_function_checks): Do not check for TYPE_ATTRIBUTES.
>
> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> index 2cfa9a6d0e9..94a80d3ec90 100644
> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -3135,10 +3135,38 @@ compute_fn_summary (struct cgraph_node *node, bool early)
>         else
>  	 info->inlinable = tree_inlinable_function_p (node->decl);
>  
> -       /* Type attributes can use parameter indices to describe them.  */
> -       if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl))
> -	   /* Likewise for #pragma omp declare simd functions or functions
> -	      with simd attribute.  */
> +       bool no_signature = false;
> +       /* Type attributes can use parameter indices to describe them.
> +	  Special case fn spec since we can safely preserve them in
> +	  modref summaries.  */
> +       for (tree list = TYPE_ATTRIBUTES (TREE_TYPE (node->decl));
> +	    list && !no_signature; list = TREE_CHAIN (list))
> +	 if (!flag_ipa_modref
> +	     || !is_attribute_p ("fn spec", get_attribute_name (list)))
> +	   {
> +	     if (dump_file)
> +		{
> +		  fprintf (dump_file, "No signature change:"
> +			   " function type has unhandled attribute %s.\n",
> +			   IDENTIFIER_POINTER (get_attribute_name (list)));
> +		}
> +	     no_signature = true;
> +	   }
> +       for (tree parm = DECL_ARGUMENTS (node->decl);
> +	    parm && !no_signature; parm = DECL_CHAIN (parm))
> +	 if (variably_modified_type_p (TREE_TYPE (parm), node->decl))
> +	   {
> +	     if (dump_file)
> +		{
> +		  fprintf (dump_file, "No signature change:"
> +			   " has parameter with variably modified type.\n");
> +		}
> +	     no_signature = true;
> +	   }
> +
> +       /* Likewise for #pragma omp declare simd functions or functions
> +	  with simd attribute.  */
> +       if (no_signature
>  	   || lookup_attribute ("omp declare simd",
>  				DECL_ATTRIBUTES (node->decl)))
>  	 node->can_change_signature = false;
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index ae3149718ca..20f41dd5363 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "symtab-clones.h"
>  #include "tree-phinodes.h"
>  #include "cfgexpand.h"
> +#include "attribs.h"
>  
>  
>  /* Actual prefixes of different newly synthetized parameters.  Keep in sync
> @@ -281,11 +282,13 @@ fill_vector_of_new_param_types (vec<tree> *new_types, vec<tree> *otypes,
>  /* Build and return a function type just like ORIG_TYPE but with parameter
>     types given in NEW_PARAM_TYPES - which can be NULL if, but only if,
>     ORIG_TYPE itself has NULL TREE_ARG_TYPEs.  If METHOD2FUNC is true, also make
> -   it a FUNCTION_TYPE instead of FUNCTION_TYPE.  */
> +   it a FUNCTION_TYPE instead of FUNCTION_TYPE.
> +   If ARG_MODIFIED is true drop attributes that are no longer up to date.  */
>  
>  static tree
>  build_adjusted_function_type (tree orig_type, vec<tree> *new_param_types,
> -			      bool method2func, bool skip_return)
> +			      bool method2func, bool skip_return,
> +			      bool args_modified)
>  {
>    tree new_arg_types = NULL;
>    if (TYPE_ARG_TYPES (orig_type))
> @@ -334,6 +337,17 @@ build_adjusted_function_type (tree orig_type, vec<tree> *new_param_types,
>        if (skip_return)
>  	TREE_TYPE (new_type) = void_type_node;
>      }
> +  /* We only support one fn spec attribute on type.  Be sure to remove it.
> +     Once we support multiple attributes we will need to be able to unshare
> +     the list.  */
> +  if (args_modified && TYPE_ATTRIBUTES (new_type))
> +    {
> +      gcc_checking_assert
> +	      (!TREE_CHAIN (TYPE_ATTRIBUTES (new_type))
> +	       && (is_attribute_p ("fn spec",
> +			  get_attribute_name (TYPE_ATTRIBUTES (new_type)))));
> +      TYPE_ATTRIBUTES (new_type) = NULL;
> +    }
>  
>    return new_type;
>  }
> @@ -460,8 +474,22 @@ ipa_param_adjustments::build_new_function_type (tree old_type,
>    else
>      new_param_types_p = NULL;
>  
> +  /* Check if any params type cares about are modified.  In this case will
> +     need to drop some type attributes.  */
> +  bool modified = false;
> +  size_t index = 0;
> +  if (m_adj_params)
> +    for (tree t = TYPE_ARG_TYPES (old_type);
> +	 t && (int)index < m_always_copy_start && !modified;
> +	 t = TREE_CHAIN (t), index++)
> +      if (index >= m_adj_params->length ()
> +	  || get_original_index (index) != (int)index)
> +	modified = true;
> +
> +
>    return build_adjusted_function_type (old_type, new_param_types_p,
> -				       method2func_p (old_type), m_skip_return);
> +				       method2func_p (old_type), m_skip_return,
> +				       modified);
>  }
>  
>  /* Build variant of function decl ORIG_DECL which has no return value if
> @@ -1467,12 +1495,23 @@ ipa_param_body_adjustments::modify_formal_parameters ()
>    if (fndecl_built_in_p (m_fndecl))
>      set_decl_built_in_function (m_fndecl, NOT_BUILT_IN, 0);
>  
> +  bool modified = false;
> +  size_t index = 0;
> +  if (m_adj_params)
> +    for (tree t = TYPE_ARG_TYPES (orig_type);
> +	 t && !modified;
> +	 t = TREE_CHAIN (t), index++)
> +      if (index >= m_adj_params->length ()
> +	  || (*m_adj_params)[index].op != IPA_PARAM_OP_COPY
> +	  || (*m_adj_params)[index].base_index != index)
> +	modified = true;
> +
>    /* At this point, removing return value is only implemented when going
>       through tree_function_versioning, not when modifying function body
>       directly.  */
>    gcc_assert (!m_adjustments || !m_adjustments->m_skip_return);
>    tree new_type = build_adjusted_function_type (orig_type, &m_new_types,
> -						m_method2func, false);
> +						m_method2func, false, modified);
>  
>    TREE_TYPE (m_fndecl) = new_type;
>    DECL_VIRTUAL_P (m_fndecl) = 0;
> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
> index 88036590425..cb0e30507a1 100644
> --- a/gcc/ipa-sra.c
> +++ b/gcc/ipa-sra.c
> @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-streamer.h"
>  #include "internal-fn.h"
>  #include "symtab-clones.h"
> +#include "attribs.h"
>  
>  static void ipa_sra_summarize_function (cgraph_node *);
>  
> @@ -616,13 +617,6 @@ ipa_sra_preliminary_function_checks (cgraph_node *node)
>        return false;
>      }
>  
> -  if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl)))
> -    {
> -      if (dump_file)
> -	fprintf (dump_file, "Function type has attributes. \n");
> -      return false;
> -    }
> -
>    if (DECL_DISREGARD_INLINE_LIMITS (node->decl))
>      {
>        if (dump_file)