Re: Enable more type attributes for signature changes

[Martin Sebor <msebor@gmail.com>]

On 11/13/21 7:44 AM, Jan Hubicka wrote:
> Hi,
> this patch whitelists attributes that are safe for attribute changes and
> also makes access attribute dropped if function sigunature is changed.
> We could do better by updating the attribute, but doing so seems to be
> bit snowballing since with LTO the warnings produced seems bit confused.
> We would also like to output original name of function
> instead of mangledname.constprop or so.  I looked into what attributes
> are dorpped in bootstrap and it does not look too bad.
> 
> Bootstrapped/regtested x86_64-linux, will commit it shortly.

I've always intended the access attribute to eventually benefit
optimization so please feel free (and encouraged :) to use it
for that purpose.

Martin

> 
> Honza
> 
> gcc/ChangeLog:
> 
> 	* ipa-fnsummary.c (compute_fn_summary): Use type_attribut_allowed_p
> 	* ipa-param-manipulation.c (ipa_param_adjustments::type_attribute_allowed_p):
> 	New member function.
> 	(drop_type_attribute_if_params_changed_p): New function.
> 	(build_adjusted_function_type): Use it.
> 	* ipa-param-manipulation.h: Add type_attribute_allowed_p.
> 
> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> index 94a80d3ec90..7e9201a554a 100644
> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -3141,8 +3141,8 @@ compute_fn_summary (struct cgraph_node *node, bool early)
>   	  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 (!ipa_param_adjustments::type_attribute_allowed_p
> +			(get_attribute_name (list)))
>   	   {
>   	     if (dump_file)
>   		{
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index 991db0d9b1b..29268fa5a58 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -279,6 +279,32 @@ fill_vector_of_new_param_types (vec<tree> *new_types, vec<tree> *otypes,
>       }
>   }
>   
> +/* Return false if given attribute should prevent type adjustments.  */
> +
> +bool
> +ipa_param_adjustments::type_attribute_allowed_p (tree name)
> +{
> +  if ((is_attribute_p ("fn spec", name) && flag_ipa_modref)
> +      || is_attribute_p ("access", name)
> +      || is_attribute_p ("returns_nonnull", name)
> +      || is_attribute_p ("assume_aligned", name)
> +      || is_attribute_p ("nocf_check", name)
> +      || is_attribute_p ("warn_unused_result", name))
> +    return true;
> +  return false;
> +}
> +
> +/* Return true if attribute should be dropped if parameter changed.  */
> +
> +static bool
> +drop_type_attribute_if_params_changed_p (tree name)
> +{
> +  if (is_attribute_p ("fn spec", name)
> +      || is_attribute_p ("access", name))
> +    return true;
> +  return false;
> +}
> +
>   /* 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
> @@ -337,16 +363,19 @@ 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)))));
> +      tree t = TYPE_ATTRIBUTES (new_type);
> +      tree *last = &TYPE_ATTRIBUTES (new_type);
>         TYPE_ATTRIBUTES (new_type) = NULL;
> +      for (;t; t = TREE_CHAIN (t))
> +	if (!drop_type_attribute_if_params_changed_p
> +		(get_attribute_name (t)))
> +	  {
> +	    *last = copy_node (t);
> +	    TREE_CHAIN (*last) = NULL;
> +	    last = &TREE_CHAIN (*last);
> +	  }
>       }
>   
>     return new_type;
> diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
> index 9440cbfc56c..5adf8a22356 100644
> --- a/gcc/ipa-param-manipulation.h
> +++ b/gcc/ipa-param-manipulation.h
> @@ -254,6 +254,7 @@ public:
>     /* If true, make the function not return any value.  */
>     bool m_skip_return;
>   
> +  static bool type_attribute_allowed_p (tree);
>   private:
>     ipa_param_adjustments () {}
>   
>