diff mbox series

[v1,04/16] Remove unecessary `record` argument from maybe_version_functions.

Message ID 20250203130421.2192732-5-alfie.richards@arm.com
State New
Headers
Series None |

Commit Message

Alfie Richards Feb. 3, 2025, 1:04 p.m. UTC 
  The `record` argument in maybe_version_function was intended to allow
controlling recording the relationship of versions. However, it only
exercised this if both input funcitons were already marked as versioned,
and this same logic is repeated in maybe_version_function itself so the
argument is unecessary.

gcc/cp/ChangeLog:

	* class.cc (add_method): Remove argument.
	* cp-tree.h (maybe_version_functions): Ditto.
	* decl.cc (decls_match): Ditto.
	(maybe_version_functions): Ditto.
---
 gcc/cp/class.cc  | 2 +-
 gcc/cp/cp-tree.h | 2 +-
 gcc/cp/decl.cc   | 9 +++------
 3 files changed, 5 insertions(+), 8 deletions(-)

Comments

Andrew Carlotti Feb. 7, 2025, 6:53 p.m. UTC | #1 
On Mon, Feb 03, 2025 at 01:04:08PM +0000, Alfie Richards wrote:
> 
> The `record` argument in maybe_version_function was intended to allow
> controlling recording the relationship of versions. However, it only
> exercised this if both input funcitons were already marked as versioned,
> and this same logic is repeated in maybe_version_function itself so the
> argument is unecessary.

I think this commit message is inaccurate, but it's quite confusing to me. How
about the following instead:

Previously, the `record` argument in maybe_version_function allowed the call to
cgraph_node::record_function_versions to be skipped.  However, this was only
skipped when both decls were already marked as versioned, in which case we will 
now trigger the early exit in record_function_versions instead.

The patch otherwise looks fine to me.

> 
> gcc/cp/ChangeLog:
> 
> 	* class.cc (add_method): Remove argument.
> 	* cp-tree.h (maybe_version_functions): Ditto.
> 	* decl.cc (decls_match): Ditto.
> 	(maybe_version_functions): Ditto.
> ---
>  gcc/cp/class.cc  | 2 +-
>  gcc/cp/cp-tree.h | 2 +-
>  gcc/cp/decl.cc   | 9 +++------
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 

> diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
> index f2f81a44718..a9a80d1b4be 100644
> --- a/gcc/cp/class.cc
> +++ b/gcc/cp/class.cc
> @@ -1402,7 +1402,7 @@ add_method (tree type, tree method, bool via_using)
>        /* If these are versions of the same function, process and
>  	 move on.  */
>        if (TREE_CODE (fn) == FUNCTION_DECL
> -	  && maybe_version_functions (method, fn, true))
> +	  && maybe_version_functions (method, fn))
>  	continue;
>  
>        if (DECL_INHERITED_CTOR (method))
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index ec976928f5f..8eba8d455be 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7114,7 +7114,7 @@ extern void determine_local_discriminator	(tree, tree = NULL_TREE);
>  extern bool member_like_constrained_friend_p	(tree);
>  extern bool fns_correspond			(tree, tree);
>  extern int decls_match				(tree, tree, bool = true);
> -extern bool maybe_version_functions		(tree, tree, bool);
> +extern bool maybe_version_functions		(tree, tree);
>  extern bool validate_constexpr_redeclaration	(tree, tree);
>  extern bool merge_default_template_args		(tree, tree, bool);
>  extern tree duplicate_decls			(tree, tree,
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index cf5e055e146..3b3b4481964 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -1215,9 +1215,7 @@ decls_match (tree newdecl, tree olddecl, bool record_versions /* = true */)
>  	  && targetm.target_option.function_versions (newdecl, olddecl))
>  	{
>  	  if (record_versions)
> -	    maybe_version_functions (newdecl, olddecl,
> -				     (!DECL_FUNCTION_VERSIONED (newdecl)
> -				      || !DECL_FUNCTION_VERSIONED (olddecl)));
> +	    maybe_version_functions (newdecl, olddecl);
>  	  return 0;
>  	}
>      }
> @@ -1288,7 +1286,7 @@ maybe_mark_function_versioned (tree decl)
>     If RECORD is set to true, record function versions.  */
>  
>  bool
> -maybe_version_functions (tree newdecl, tree olddecl, bool record)
> +maybe_version_functions (tree newdecl, tree olddecl)
>  {
>    if (!targetm.target_option.function_versions (newdecl, olddecl))
>      return false;
> @@ -1311,8 +1309,7 @@ maybe_version_functions (tree newdecl, tree olddecl, bool record)
>        maybe_mark_function_versioned (newdecl);
>      }
>  
> -  if (record)
> -    cgraph_node::record_function_versions (olddecl, newdecl);
> +  cgraph_node::record_function_versions (olddecl, newdecl);
>  
>    return true;
>  }
diff mbox series

Patch

diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index f2f81a44718..a9a80d1b4be 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -1402,7 +1402,7 @@  add_method (tree type, tree method, bool via_using)
       /* If these are versions of the same function, process and
 	 move on.  */
       if (TREE_CODE (fn) == FUNCTION_DECL
-	  && maybe_version_functions (method, fn, true))
+	  && maybe_version_functions (method, fn))
 	continue;
 
       if (DECL_INHERITED_CTOR (method))
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ec976928f5f..8eba8d455be 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7114,7 +7114,7 @@  extern void determine_local_discriminator	(tree, tree = NULL_TREE);
 extern bool member_like_constrained_friend_p	(tree);
 extern bool fns_correspond			(tree, tree);
 extern int decls_match				(tree, tree, bool = true);
-extern bool maybe_version_functions		(tree, tree, bool);
+extern bool maybe_version_functions		(tree, tree);
 extern bool validate_constexpr_redeclaration	(tree, tree);
 extern bool merge_default_template_args		(tree, tree, bool);
 extern tree duplicate_decls			(tree, tree,
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index cf5e055e146..3b3b4481964 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -1215,9 +1215,7 @@  decls_match (tree newdecl, tree olddecl, bool record_versions /* = true */)
 	  && targetm.target_option.function_versions (newdecl, olddecl))
 	{
 	  if (record_versions)
-	    maybe_version_functions (newdecl, olddecl,
-				     (!DECL_FUNCTION_VERSIONED (newdecl)
-				      || !DECL_FUNCTION_VERSIONED (olddecl)));
+	    maybe_version_functions (newdecl, olddecl);
 	  return 0;
 	}
     }
@@ -1288,7 +1286,7 @@  maybe_mark_function_versioned (tree decl)
    If RECORD is set to true, record function versions.  */
 
 bool
-maybe_version_functions (tree newdecl, tree olddecl, bool record)
+maybe_version_functions (tree newdecl, tree olddecl)
 {
   if (!targetm.target_option.function_versions (newdecl, olddecl))
     return false;
@@ -1311,8 +1309,7 @@  maybe_version_functions (tree newdecl, tree olddecl, bool record)
       maybe_mark_function_versioned (newdecl);
     }
 
-  if (record)
-    cgraph_node::record_function_versions (olddecl, newdecl);
+  cgraph_node::record_function_versions (olddecl, newdecl);
 
   return true;
 }