diff mbox series

Enable more type attributes for signature changes

Message ID 20211113144438.GA952@kam.mff.cuni.cz
State New
Headers show
Series Enable more type attributes for signature changes | expand

Commit Message

Jan Hubicka Nov. 13, 2021, 2:44 p.m. UTC
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.

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.

Comments

Martin Sebor Nov. 15, 2021, 3:38 p.m. UTC | #1
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 () {}
>   
>
diff mbox series

Patch

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 () {}