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;
> }
