Alfie Richards <alfie.richa...@arm.com> writes: > This adds the assembler_name member to cgraph_function_version_info > to store the base assembler name for the function to be mangled. This is > used in later patches for refactoring FMV mangling. > > gcc/c/ChangeLog: > > * c-decl.cc (start_decl): Record assembler_name. > (start_function): Record assembler_name. > > gcc/ChangeLog: > > * cgraph.cc (cgraph_node::record_function_versions): Record > assembler_name. > * cgraph.h (struct cgraph_function_version_info): Add assembler_name. > > gcc/cp/ChangeLog: > > * decl.cc (maybe_mark_function_versioned): Record assember_name. > (start_decl): Record assembler_name. > (start_preparsed_function): Record assembler_name.
I'm not really qualified to review this part, but FWIW, my main question would be: is it safe to cache the DECL_ASSEMBLER_NAME so early? E.g. wrt https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117987 , it looks like the C frontend applies the asm in finish_decl rather than start_decl. I don't know whether the name could change even later, or whether it is supposed to be final after finish_decl. Also: > --- > gcc/c/c-decl.cc | 20 ++++++++++++++++++++ > gcc/cgraph.cc | 10 ++++++++-- > gcc/cgraph.h | 3 +++ > gcc/cp/decl.cc | 34 ++++++++++++++++++++++++++++++++++ > 4 files changed, 65 insertions(+), 2 deletions(-) > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > index 0dcbae9b26f..daa19f360e6 100644 > --- a/gcc/c/c-decl.cc > +++ b/gcc/c/c-decl.cc > @@ -5762,6 +5762,16 @@ start_decl (struct c_declarator *declarator, struct > c_declspecs *declspecs, > && VAR_OR_FUNCTION_DECL_P (decl)) > objc_check_global_decl (decl); > > + /* Store the base assembler name for mangling later. */ > + if (TREE_CODE (decl) == FUNCTION_DECL > + && lookup_attribute ("target_clones", DECL_ATTRIBUTES (decl))) > + { > + cgraph_node *node = cgraph_node::get_create (decl); > + if (!node->function_version ()) > + node->insert_new_function_version (); > + node->function_version ()->assembler_name = DECL_ASSEMBLER_NAME (decl); > + } > + > /* Add this decl to the current scope. > TEM may equal DECL or it may be a previous decl of the same name. */ > if (do_push) > @@ -10863,6 +10873,16 @@ start_function (struct c_declspecs *declspecs, > struct c_declarator *declarator, > > current_function_decl = pushdecl (decl1); > > + /* Store the base assembler name for mangling later. */ > + if (TREE_CODE (decl1) == FUNCTION_DECL > + && lookup_attribute ("target_clones", DECL_ATTRIBUTES (decl1))) > + { > + cgraph_node *node = cgraph_node::get_create (decl1); > + if (!node->function_version ()) > + node->insert_new_function_version (); > + node->function_version ()->assembler_name = DECL_ASSEMBLER_NAME > (decl1); > + } > + > if (tree access = build_attr_access_from_parms (parms, false)) > decl_attributes (¤t_function_decl, access, ATTR_FLAG_INTERNAL, > old_decl); > diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc > index 1ea38d16e56..c2038be4671 100644 > --- a/gcc/cgraph.cc > +++ b/gcc/cgraph.cc > @@ -252,10 +252,16 @@ cgraph_node::record_function_versions (tree decl1, tree > decl2) > decl2_v = decl2_node->function_version (); > > if (decl1_v == NULL) > - decl1_v = decl1_node->insert_new_function_version (); > + { > + decl1_v = decl1_node->insert_new_function_version (); > + decl1_v->assembler_name = DECL_ASSEMBLER_NAME (decl1); > + } > > if (decl2_v == NULL) > - decl2_v = decl2_node->insert_new_function_version (); > + { > + decl2_v = decl2_node->insert_new_function_version (); > + decl2_v->assembler_name = DECL_ASSEMBLER_NAME (decl2); > + } > > gcc_assert (decl1_v); > gcc_assert (decl2_v); > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > index 065fcc742e8..d9177364b7a 100644 > --- a/gcc/cgraph.h > +++ b/gcc/cgraph.h > @@ -856,6 +856,9 @@ struct GTY((for_user)) cgraph_function_version_info { > dispatcher. The dispatcher decl is an alias to the resolver > function decl. */ > tree dispatcher_resolver; > + > + /* The assmbly name of the function set before version mangling. */ > + tree assembler_name; > }; > > #define DEFCIFCODE(code, type, string) CIF_ ## code, > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 3b3b4481964..fdef98f8062 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -1273,6 +1273,12 @@ maybe_mark_function_versioned (tree decl) > { > if (!DECL_FUNCTION_VERSIONED (decl)) > { > + cgraph_node *node = cgraph_node::get_create (decl); > + if (!node->function_version ()) > + node->insert_new_function_version (); > + if (!node->function_version ()->assembler_name) > + node->function_version ()->assembler_name = DECL_ASSEMBLER_NAME (decl); > + > DECL_FUNCTION_VERSIONED (decl) = 1; > /* If DECL_ASSEMBLER_NAME has already been set, re-mangle > to include the version marker. */ > @@ -6155,6 +6161,20 @@ start_decl (const cp_declarator *declarator, > > was_public = TREE_PUBLIC (decl); > > + /* Set the assembler string for any versioned function. */ > + if (TREE_CODE (decl) == FUNCTION_DECL > + && (lookup_attribute (TARGET_HAS_FMV_TARGET_ATTRIBUTE ? "target" > + : "target_version", > + DECL_ATTRIBUTES (decl)) > + || lookup_attribute ("target_clones", DECL_ATTRIBUTES (decl)))) > + { > + cgraph_node *node = cgraph_node::get_create (decl); > + if (!node->function_version ()) > + node->insert_new_function_version (); > + if (!node->function_version ()->assembler_name) > + node->function_version ()->assembler_name = DECL_ASSEMBLER_NAME (decl); > + } function_version () is a hash lookup, so I think we should try to minimise the number of calls. E.g.: auto *decl_version = node->function_version (); if (!decl_version) decl_version = node->insert_new_function_version (); It'd also be good avoid the cut-&-paste, in the sense that the C frontend has two copies of the same code, and similarly the C++ frontend. Thanks, Richard > + > if ((DECL_EXTERNAL (decl) || TREE_CODE (decl) == FUNCTION_DECL) > && current_function_decl) > { > @@ -18743,6 +18763,20 @@ start_preparsed_function (tree decl1, tree attrs, > int flags) > if (!DECL_OMP_DECLARE_REDUCTION_P (decl1)) > start_lambda_scope (decl1); > > + /* Set the assembler string for any versioned function. */ > + if (TREE_CODE (decl1) == FUNCTION_DECL > + && (lookup_attribute (TARGET_HAS_FMV_TARGET_ATTRIBUTE ? "target" > + : "target_version", > + DECL_ATTRIBUTES (decl1)) > + || lookup_attribute ("target_clones", DECL_ATTRIBUTES (decl1)))) > + { > + cgraph_node *node = cgraph_node::get_create (decl1); > + if (!node->function_version ()) > + node->insert_new_function_version (); > + if (!node->function_version ()->assembler_name) > + node->function_version ()->assembler_name = DECL_ASSEMBLER_NAME (decl1); > + } > + > return true; > } >
