[1/3] Add support for target_version attribute
Checks
Commit Message
This patch adds support for the "target_version" attribute to the middle
end and the C++ frontend, which will be used to implement function
multiversioning in the aarch64 backend.
Note that C++ is currently the only frontend which supports
multiversioning using the "target" attribute, whereas the
"target_clones" attribute is additionally supported in C, D and Ada.
Support for the target_version attribute will be extended to C at a
later date.
Targets that currently use the "target" attribute for function
multiversioning (i.e. i386 and rs6000) are not affected by this patch.
I could have implemented the target hooks slightly differently, by reusing the
valid_attribute_p hook and adding attribute name checks to each backend
implementation (c.f. the aarch64 implementation in patch 2/3). Would this be
preferable?
Otherwise, is this ok for master?
gcc/c-family/ChangeLog:
* c-attribs.cc (handle_target_version_attribute): New.
(c_common_attribute_table): Add target_version.
(handle_target_clones_attribute): Add conflict with
target_version attribute.
gcc/ChangeLog:
* attribs.cc (is_function_default_version): Update comment to
specify incompatibility with target_version attributes.
* cgraphclones.cc (cgraph_node::create_version_clone_with_body):
Call valid_version_attribute_p for target_version attributes.
* target.def (valid_version_attribute_p): New hook.
(expanded_clones_attribute): New hook.
* doc/tm.texi.in: Add new hooks.
* doc/tm.texi: Regenerate.
* multiple_target.cc (create_dispatcher_calls): Remove redundant
is_function_default_version check.
(expand_target_clones): Use target hook for attribute name.
* targhooks.cc (default_target_option_valid_version_attribute_p):
New.
* targhooks.h (default_target_option_valid_version_attribute_p):
New.
* tree.h (DECL_FUNCTION_VERSIONED): Update comment to include
target_version attributes.
gcc/cp/ChangeLog:
* decl2.cc (check_classfn): Update comment to include
target_version attributes.
Comments
On Wed, 18 Oct 2023, Andrew Carlotti wrote:
> This patch adds support for the "target_version" attribute to the middle
> end and the C++ frontend, which will be used to implement function
> multiversioning in the aarch64 backend.
>
> Note that C++ is currently the only frontend which supports
> multiversioning using the "target" attribute, whereas the
> "target_clones" attribute is additionally supported in C, D and Ada.
> Support for the target_version attribute will be extended to C at a
> later date.
>
> Targets that currently use the "target" attribute for function
> multiversioning (i.e. i386 and rs6000) are not affected by this patch.
>
>
> I could have implemented the target hooks slightly differently, by reusing the
> valid_attribute_p hook and adding attribute name checks to each backend
> implementation (c.f. the aarch64 implementation in patch 2/3). Would this be
> preferable?
>
> Otherwise, is this ok for master?
This lacks user-level documentation in doc/extend.texi (where
target_clones is documented).
Was there any discussion/description of why target_clones cannot
be made work for aarch64?
Richard.
>
> gcc/c-family/ChangeLog:
>
> * c-attribs.cc (handle_target_version_attribute): New.
> (c_common_attribute_table): Add target_version.
> (handle_target_clones_attribute): Add conflict with
> target_version attribute.
>
> gcc/ChangeLog:
>
> * attribs.cc (is_function_default_version): Update comment to
> specify incompatibility with target_version attributes.
> * cgraphclones.cc (cgraph_node::create_version_clone_with_body):
> Call valid_version_attribute_p for target_version attributes.
> * target.def (valid_version_attribute_p): New hook.
> (expanded_clones_attribute): New hook.
> * doc/tm.texi.in: Add new hooks.
> * doc/tm.texi: Regenerate.
> * multiple_target.cc (create_dispatcher_calls): Remove redundant
> is_function_default_version check.
> (expand_target_clones): Use target hook for attribute name.
> * targhooks.cc (default_target_option_valid_version_attribute_p):
> New.
> * targhooks.h (default_target_option_valid_version_attribute_p):
> New.
> * tree.h (DECL_FUNCTION_VERSIONED): Update comment to include
> target_version attributes.
>
> gcc/cp/ChangeLog:
>
> * decl2.cc (check_classfn): Update comment to include
> target_version attributes.
>
>
> diff --git a/gcc/attribs.cc b/gcc/attribs.cc
> index b1300018d1e8ed8e02ded1ea721dc192a6d32a49..a3c4a81e8582ea4fd06b9518bf51fad7c998ddd6 100644
> --- a/gcc/attribs.cc
> +++ b/gcc/attribs.cc
> @@ -1233,8 +1233,9 @@ make_dispatcher_decl (const tree decl)
> return func_decl;
> }
>
> -/* Returns true if decl is multi-versioned and DECL is the default function,
> - that is it is not tagged with target specific optimization. */
> +/* Returns true if DECL is multi-versioned using the target attribute, and this
> + is the default version. This function can only be used for targets that do
> + not support the "target_version" attribute. */
>
> bool
> is_function_default_version (const tree decl)
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 072cfb69147bd6b314459c0bd48a0c1fb92d3e4d..1a224c036277d51ab4dc0d33a403177bd226e48a 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -148,6 +148,7 @@ static tree handle_alloc_align_attribute (tree *, tree, tree, int, bool *);
> static tree handle_assume_aligned_attribute (tree *, tree, tree, int, bool *);
> static tree handle_assume_attribute (tree *, tree, tree, int, bool *);
> static tree handle_target_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_target_version_attribute (tree *, tree, tree, int, bool *);
> static tree handle_target_clones_attribute (tree *, tree, tree, int, bool *);
> static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
> static tree ignore_attribute (tree *, tree, tree, int, bool *);
> @@ -480,6 +481,8 @@ const struct attribute_spec c_common_attribute_table[] =
> handle_error_attribute, NULL },
> { "target", 1, -1, true, false, false, false,
> handle_target_attribute, NULL },
> + { "target_version", 1, -1, true, false, false, false,
> + handle_target_version_attribute, NULL },
> { "target_clones", 1, -1, true, false, false, false,
> handle_target_clones_attribute, NULL },
> { "optimize", 1, -1, true, false, false, false,
> @@ -5569,6 +5572,45 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
> return NULL_TREE;
> }
>
> +/* Handle a "target_version" attribute. */
> +
> +static tree
> +handle_target_version_attribute (tree *node, tree name, tree args, int flags,
> + bool *no_add_attrs)
> +{
> + /* Ensure we have a function type. */
> + if (TREE_CODE (*node) != FUNCTION_DECL)
> + {
> + warning (OPT_Wattributes, "%qE attribute ignored", name);
> + *no_add_attrs = true;
> + }
> + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (*node)))
> + {
> + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> + "with %qs attribute", name, "target_clones");
> + *no_add_attrs = true;
> + }
> + else if (!targetm.target_option.valid_version_attribute_p (*node, name, args,
> + flags))
> + *no_add_attrs = true;
> +
> + /* Check that there's no empty string in values of the attribute. */
> + for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
> + {
> + tree value = TREE_VALUE (t);
> + if (TREE_CODE (value) == STRING_CST
> + && TREE_STRING_LENGTH (value) == 1
> + && TREE_STRING_POINTER (value)[0] == '\0')
> + {
> + warning (OPT_Wattributes,
> + "empty string in attribute %<target_version%>");
> + *no_add_attrs = true;
> + }
> + }
> +
> + return NULL_TREE;
> +}
> +
> /* Handle a "target_clones" attribute. */
>
> static tree
> @@ -5601,6 +5643,12 @@ handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
> "with %qs attribute", name, "target");
> *no_add_attrs = true;
> }
> + else if (lookup_attribute ("target_version", DECL_ATTRIBUTES (*node)))
> + {
> + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> + "with %qs attribute", name, "target_version");
> + *no_add_attrs = true;
> + }
> else if (get_target_clone_attr_len (args) == -1)
> {
> warning (OPT_Wattributes,
> diff --git a/gcc/cgraphclones.cc b/gcc/cgraphclones.cc
> index 29d28ef895a73a223695cbb86aafbc845bbe7688..8af6b23d8c0306920e0fdcb3559ef047a16689f4 100644
> --- a/gcc/cgraphclones.cc
> +++ b/gcc/cgraphclones.cc
> @@ -78,6 +78,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-eh.h"
> #include "tree-cfg.h"
> #include "tree-inline.h"
> +#include "attribs.h"
> #include "dumpfile.h"
> #include "gimple-pretty-print.h"
> #include "alloc-pool.h"
> @@ -1048,7 +1049,17 @@ cgraph_node::create_version_clone_with_body
> location_t saved_loc = input_location;
> tree v = TREE_VALUE (target_attributes);
> input_location = DECL_SOURCE_LOCATION (new_decl);
> - bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
> + bool r;
> + tree name_id = get_attribute_name (target_attributes);
> + const char* name_str = IDENTIFIER_POINTER (name_id);
> + if (strcmp (name_str, "target") == 0)
> + r = targetm.target_option.valid_attribute_p (new_decl, name_id, v, 1);
> + else if (strcmp (name_str, "target_version") == 0)
> + r = targetm.target_option.valid_version_attribute_p (new_decl, name_id,
> + v, 1);
> + else
> + gcc_assert(false);
> +
> input_location = saved_loc;
> if (!r)
> return NULL;
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 9594be4092c3c00fddc9d4c6da5931ea3b7e8792..ec78d5a5440bedd360ac8e5bc44e164da3dab410 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -829,8 +829,8 @@ check_classfn (tree ctype, tree function, tree template_parms)
> tree c2 = get_constraints (fndecl);
>
> /* While finding a match, same types and params are not enough
> - if the function is versioned. Also check version ("target")
> - attributes. */
> + if the function is versioned. Also check for different target
> + specific attributes. */
> if (same_type_p (TREE_TYPE (TREE_TYPE (function)),
> TREE_TYPE (TREE_TYPE (fndecl)))
> && compparms (p1, p2)
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 33e7ffc8af5a9d48430145ef9b8e7924613b7dd7..97b3e9c31631a943d95f1cf7739716a574afcfb7 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -10507,6 +10507,23 @@ the function declaration to hold a pointer to a target-specific
> @code{struct cl_target_option} structure.
> @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P (tree @var{fndecl}, tree @var{name}, tree @var{args}, int @var{flags})
> +This hook is called to parse @code{attribute(target_version("..."))},
> +which allows setting target-specific options on individual function versions.
> +These function-specific options may differ
> +from the options specified on the command line. The hook should return
> +@code{true} if the options are valid.
> +
> +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in
> +the function declaration to hold a pointer to a target-specific
> +@code{struct cl_target_option} structure.
> +@end deftypefn
> +
> +@deftypevr {Target Hook} {const char *} TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> +Contains the name of the attribute used for the version description string
> +when expanding clones for a function with the target_clones attribute.
> +@end deftypevr
> +
> @deftypefn {Target Hook} void TARGET_OPTION_SAVE (struct cl_target_option *@var{ptr}, struct gcc_options *@var{opts}, struct gcc_options *@var{opts_set})
> This hook is called to save any additional target-specific information
> in the @code{struct cl_target_option} structure for function-specific
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index c98b2447e28aa17996b1cbf8af7ed02d70db54f2..56fa3de6bba06bc0ac124bb3a41324be1997e209 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -6979,6 +6979,10 @@ on this implementation detail.
>
> @hook TARGET_OPTION_VALID_ATTRIBUTE_P
>
> +@hook TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P
> +
> +@hook TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> +
> @hook TARGET_OPTION_SAVE
>
> @hook TARGET_OPTION_RESTORE
> diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
> index a2ed048d7dd28ec470953fcd8a0dc86817e4b7dc..3db57c2b13d612a37240d9dcf58ad21b2286633c 100644
> --- a/gcc/multiple_target.cc
> +++ b/gcc/multiple_target.cc
> @@ -66,10 +66,6 @@ create_dispatcher_calls (struct cgraph_node *node)
> {
> ipa_ref *ref;
>
> - if (!DECL_FUNCTION_VERSIONED (node->decl)
> - || !is_function_default_version (node->decl))
> - return;
> -
> if (!targetm.has_ifunc_p ())
> {
> error_at (DECL_SOURCE_LOCATION (node->decl),
> @@ -377,6 +373,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> return false;
> }
>
> + const char *new_attr_name = targetm.target_option.expanded_clones_attribute;
> cgraph_function_version_info *decl1_v = NULL;
> cgraph_function_version_info *decl2_v = NULL;
> cgraph_function_version_info *before = NULL;
> @@ -392,7 +389,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> char *attr = attrs[i];
>
> /* Create new target clone. */
> - tree attributes = make_attribute ("target", attr,
> + tree attributes = make_attribute (new_attr_name, attr,
> DECL_ATTRIBUTES (node->decl));
>
> char *suffix = XNEWVEC (char, strlen (attr) + 1);
> @@ -430,7 +427,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> XDELETEVEC (attr_str);
>
> /* Setting new attribute to initial function. */
> - tree attributes = make_attribute ("target", "default",
> + tree attributes = make_attribute (new_attr_name, "default",
> DECL_ATTRIBUTES (node->decl));
> DECL_ATTRIBUTES (node->decl) = attributes;
> node->local = false;
> diff --git a/gcc/target.def b/gcc/target.def
> index cda6c51e5167f85625168c7c26b777d6c8ccad82..39acea04db01ebaf918910b7dd73d397de6a84ec 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -6492,6 +6492,31 @@ the function declaration to hold a pointer to a target-specific\n\
> bool, (tree fndecl, tree name, tree args, int flags),
> default_target_option_valid_attribute_p)
>
> +/* Function to validate the attribute((target_version(...))) strings. If
> + the option is validated, the hook should also fill in
> + DECL_FUNCTION_SPECIFIC_TARGET in the function decl node. */
> +DEFHOOK
> +(valid_version_attribute_p,
> + "This hook is called to parse @code{attribute(target_version(\"...\"))},\n\
> +which allows setting target-specific options on individual function versions.\n\
> +These function-specific options may differ\n\
> +from the options specified on the command line. The hook should return\n\
> +@code{true} if the options are valid.\n\
> +\n\
> +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in\n\
> +the function declaration to hold a pointer to a target-specific\n\
> +@code{struct cl_target_option} structure.",
> + bool, (tree fndecl, tree name, tree args, int flags),
> + default_target_option_valid_version_attribute_p)
> +
> +/* Attribute to be used when expanding clones for functions with
> + target_clones attribute. */
> +DEFHOOKPOD
> +(expanded_clones_attribute,
> + "Contains the name of the attribute used for the version description string\n\
> +when expanding clones for a function with the target_clones attribute.",
> + const char *, "target")
> +
> /* Function to save any extra target state in the target options structure. */
> DEFHOOK
> (save,
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 1a0db8dddd594d9b1fb04ae0d9a66ad6b7a396dc..0efc993d82ef59b581a1df74ee0de71135a28703 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -192,6 +192,7 @@ extern bool default_hard_regno_scratch_ok (unsigned int);
> extern bool default_mode_dependent_address_p (const_rtx, addr_space_t);
> extern bool default_new_address_profitable_p (rtx, rtx_insn *, rtx);
> extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
> +extern bool default_target_option_valid_version_attribute_p (tree, tree, tree, int);
> extern bool default_target_option_pragma_parse (tree, tree);
> extern bool default_target_can_inline_p (tree, tree);
> extern bool default_update_ipa_fn_target_info (unsigned int &, const gimple *);
> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> index e190369f87a92e6a92372dc348d9374c3a965c0a..7fc7bf455e80c333cced1bac7085210c2b108f8d 100644
> --- a/gcc/targhooks.cc
> +++ b/gcc/targhooks.cc
> @@ -1787,7 +1787,19 @@ default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl),
> int ARG_UNUSED (flags))
> {
> warning (OPT_Wattributes,
> - "target attribute is not supported on this machine");
> + "%<target%> attribute is not supported on this machine");
> +
> + return false;
> +}
> +
> +bool
> +default_target_option_valid_version_attribute_p (tree ARG_UNUSED (fndecl),
> + tree ARG_UNUSED (name),
> + tree ARG_UNUSED (args),
> + int ARG_UNUSED (flags))
> +{
> + warning (OPT_Wattributes,
> + "%<target_version%> attribute is not supported on this machine");
>
> return false;
> }
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0b72663e6a1a94406127f6253460f498b7a3ea9c..ebd89ce79566c350eaaab210c0dca3cc1ac2048e 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -3438,8 +3438,8 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree);
> (FUNCTION_DECL_CHECK (NODE)->function_decl.function_specific_optimization)
>
> /* In FUNCTION_DECL, this is set if this function has other versions generated
> - using "target" attributes. The default version is the one which does not
> - have any "target" attribute set. */
> + to support different architecture feature sets, e.g. using "target" or
> + "target_version" attributes. */
> #define DECL_FUNCTION_VERSIONED(NODE)\
> (FUNCTION_DECL_CHECK (NODE)->function_decl.versioned_function)
>
>
On Thu, Oct 19, 2023 at 07:04:09AM +0000, Richard Biener wrote:
> On Wed, 18 Oct 2023, Andrew Carlotti wrote:
>
> > This patch adds support for the "target_version" attribute to the middle
> > end and the C++ frontend, which will be used to implement function
> > multiversioning in the aarch64 backend.
> >
> > Note that C++ is currently the only frontend which supports
> > multiversioning using the "target" attribute, whereas the
> > "target_clones" attribute is additionally supported in C, D and Ada.
> > Support for the target_version attribute will be extended to C at a
> > later date.
> >
> > Targets that currently use the "target" attribute for function
> > multiversioning (i.e. i386 and rs6000) are not affected by this patch.
> >
> >
> > I could have implemented the target hooks slightly differently, by reusing the
> > valid_attribute_p hook and adding attribute name checks to each backend
> > implementation (c.f. the aarch64 implementation in patch 2/3). Would this be
> > preferable?
> >
> > Otherwise, is this ok for master?
>
> This lacks user-level documentation in doc/extend.texi (where
> target_clones is documented).
Good point. I'll add documentation updates as a separate patch in the series
(rather than documenting the state after this patch, in which the attribute is
supported on zero targets). I think the existing documentation for target and
target_clones needs some improvement as well.
> Was there any discussion/description of why target_clones cannot
> be made work for aarch64?
>
> Richard.
The second patch in this series does include support for target_clones on
aarch64. However, the support in that patch is not fully compliant with our
ACLE specification. I also have some unresolved questions about the
correctness of current function multiversioning implementations using ifuncs
across translation units, which could affect how we want to implement it for
aarch64.
Andrew
> >
> > gcc/c-family/ChangeLog:
> >
> > * c-attribs.cc (handle_target_version_attribute): New.
> > (c_common_attribute_table): Add target_version.
> > (handle_target_clones_attribute): Add conflict with
> > target_version attribute.
> >
> > gcc/ChangeLog:
> >
> > * attribs.cc (is_function_default_version): Update comment to
> > specify incompatibility with target_version attributes.
> > * cgraphclones.cc (cgraph_node::create_version_clone_with_body):
> > Call valid_version_attribute_p for target_version attributes.
> > * target.def (valid_version_attribute_p): New hook.
> > (expanded_clones_attribute): New hook.
> > * doc/tm.texi.in: Add new hooks.
> > * doc/tm.texi: Regenerate.
> > * multiple_target.cc (create_dispatcher_calls): Remove redundant
> > is_function_default_version check.
> > (expand_target_clones): Use target hook for attribute name.
> > * targhooks.cc (default_target_option_valid_version_attribute_p):
> > New.
> > * targhooks.h (default_target_option_valid_version_attribute_p):
> > New.
> > * tree.h (DECL_FUNCTION_VERSIONED): Update comment to include
> > target_version attributes.
> >
> > gcc/cp/ChangeLog:
> >
> > * decl2.cc (check_classfn): Update comment to include
> > target_version attributes.
> >
> >
> > diff --git a/gcc/attribs.cc b/gcc/attribs.cc
> > index b1300018d1e8ed8e02ded1ea721dc192a6d32a49..a3c4a81e8582ea4fd06b9518bf51fad7c998ddd6 100644
> > --- a/gcc/attribs.cc
> > +++ b/gcc/attribs.cc
> > @@ -1233,8 +1233,9 @@ make_dispatcher_decl (const tree decl)
> > return func_decl;
> > }
> >
> > -/* Returns true if decl is multi-versioned and DECL is the default function,
> > - that is it is not tagged with target specific optimization. */
> > +/* Returns true if DECL is multi-versioned using the target attribute, and this
> > + is the default version. This function can only be used for targets that do
> > + not support the "target_version" attribute. */
> >
> > bool
> > is_function_default_version (const tree decl)
> > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> > index 072cfb69147bd6b314459c0bd48a0c1fb92d3e4d..1a224c036277d51ab4dc0d33a403177bd226e48a 100644
> > --- a/gcc/c-family/c-attribs.cc
> > +++ b/gcc/c-family/c-attribs.cc
> > @@ -148,6 +148,7 @@ static tree handle_alloc_align_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_assume_aligned_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_assume_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_target_attribute (tree *, tree, tree, int, bool *);
> > +static tree handle_target_version_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_target_clones_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
> > static tree ignore_attribute (tree *, tree, tree, int, bool *);
> > @@ -480,6 +481,8 @@ const struct attribute_spec c_common_attribute_table[] =
> > handle_error_attribute, NULL },
> > { "target", 1, -1, true, false, false, false,
> > handle_target_attribute, NULL },
> > + { "target_version", 1, -1, true, false, false, false,
> > + handle_target_version_attribute, NULL },
> > { "target_clones", 1, -1, true, false, false, false,
> > handle_target_clones_attribute, NULL },
> > { "optimize", 1, -1, true, false, false, false,
> > @@ -5569,6 +5572,45 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
> > return NULL_TREE;
> > }
> >
> > +/* Handle a "target_version" attribute. */
> > +
> > +static tree
> > +handle_target_version_attribute (tree *node, tree name, tree args, int flags,
> > + bool *no_add_attrs)
> > +{
> > + /* Ensure we have a function type. */
> > + if (TREE_CODE (*node) != FUNCTION_DECL)
> > + {
> > + warning (OPT_Wattributes, "%qE attribute ignored", name);
> > + *no_add_attrs = true;
> > + }
> > + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (*node)))
> > + {
> > + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> > + "with %qs attribute", name, "target_clones");
> > + *no_add_attrs = true;
> > + }
> > + else if (!targetm.target_option.valid_version_attribute_p (*node, name, args,
> > + flags))
> > + *no_add_attrs = true;
> > +
> > + /* Check that there's no empty string in values of the attribute. */
> > + for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
> > + {
> > + tree value = TREE_VALUE (t);
> > + if (TREE_CODE (value) == STRING_CST
> > + && TREE_STRING_LENGTH (value) == 1
> > + && TREE_STRING_POINTER (value)[0] == '\0')
> > + {
> > + warning (OPT_Wattributes,
> > + "empty string in attribute %<target_version%>");
> > + *no_add_attrs = true;
> > + }
> > + }
> > +
> > + return NULL_TREE;
> > +}
> > +
> > /* Handle a "target_clones" attribute. */
> >
> > static tree
> > @@ -5601,6 +5643,12 @@ handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
> > "with %qs attribute", name, "target");
> > *no_add_attrs = true;
> > }
> > + else if (lookup_attribute ("target_version", DECL_ATTRIBUTES (*node)))
> > + {
> > + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> > + "with %qs attribute", name, "target_version");
> > + *no_add_attrs = true;
> > + }
> > else if (get_target_clone_attr_len (args) == -1)
> > {
> > warning (OPT_Wattributes,
> > diff --git a/gcc/cgraphclones.cc b/gcc/cgraphclones.cc
> > index 29d28ef895a73a223695cbb86aafbc845bbe7688..8af6b23d8c0306920e0fdcb3559ef047a16689f4 100644
> > --- a/gcc/cgraphclones.cc
> > +++ b/gcc/cgraphclones.cc
> > @@ -78,6 +78,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "tree-eh.h"
> > #include "tree-cfg.h"
> > #include "tree-inline.h"
> > +#include "attribs.h"
> > #include "dumpfile.h"
> > #include "gimple-pretty-print.h"
> > #include "alloc-pool.h"
> > @@ -1048,7 +1049,17 @@ cgraph_node::create_version_clone_with_body
> > location_t saved_loc = input_location;
> > tree v = TREE_VALUE (target_attributes);
> > input_location = DECL_SOURCE_LOCATION (new_decl);
> > - bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
> > + bool r;
> > + tree name_id = get_attribute_name (target_attributes);
> > + const char* name_str = IDENTIFIER_POINTER (name_id);
> > + if (strcmp (name_str, "target") == 0)
> > + r = targetm.target_option.valid_attribute_p (new_decl, name_id, v, 1);
> > + else if (strcmp (name_str, "target_version") == 0)
> > + r = targetm.target_option.valid_version_attribute_p (new_decl, name_id,
> > + v, 1);
> > + else
> > + gcc_assert(false);
> > +
> > input_location = saved_loc;
> > if (!r)
> > return NULL;
> > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> > index 9594be4092c3c00fddc9d4c6da5931ea3b7e8792..ec78d5a5440bedd360ac8e5bc44e164da3dab410 100644
> > --- a/gcc/cp/decl2.cc
> > +++ b/gcc/cp/decl2.cc
> > @@ -829,8 +829,8 @@ check_classfn (tree ctype, tree function, tree template_parms)
> > tree c2 = get_constraints (fndecl);
> >
> > /* While finding a match, same types and params are not enough
> > - if the function is versioned. Also check version ("target")
> > - attributes. */
> > + if the function is versioned. Also check for different target
> > + specific attributes. */
> > if (same_type_p (TREE_TYPE (TREE_TYPE (function)),
> > TREE_TYPE (TREE_TYPE (fndecl)))
> > && compparms (p1, p2)
> > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > index 33e7ffc8af5a9d48430145ef9b8e7924613b7dd7..97b3e9c31631a943d95f1cf7739716a574afcfb7 100644
> > --- a/gcc/doc/tm.texi
> > +++ b/gcc/doc/tm.texi
> > @@ -10507,6 +10507,23 @@ the function declaration to hold a pointer to a target-specific
> > @code{struct cl_target_option} structure.
> > @end deftypefn
> >
> > +@deftypefn {Target Hook} bool TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P (tree @var{fndecl}, tree @var{name}, tree @var{args}, int @var{flags})
> > +This hook is called to parse @code{attribute(target_version("..."))},
> > +which allows setting target-specific options on individual function versions.
> > +These function-specific options may differ
> > +from the options specified on the command line. The hook should return
> > +@code{true} if the options are valid.
> > +
> > +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in
> > +the function declaration to hold a pointer to a target-specific
> > +@code{struct cl_target_option} structure.
> > +@end deftypefn
> > +
> > +@deftypevr {Target Hook} {const char *} TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> > +Contains the name of the attribute used for the version description string
> > +when expanding clones for a function with the target_clones attribute.
> > +@end deftypevr
> > +
> > @deftypefn {Target Hook} void TARGET_OPTION_SAVE (struct cl_target_option *@var{ptr}, struct gcc_options *@var{opts}, struct gcc_options *@var{opts_set})
> > This hook is called to save any additional target-specific information
> > in the @code{struct cl_target_option} structure for function-specific
> > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > index c98b2447e28aa17996b1cbf8af7ed02d70db54f2..56fa3de6bba06bc0ac124bb3a41324be1997e209 100644
> > --- a/gcc/doc/tm.texi.in
> > +++ b/gcc/doc/tm.texi.in
> > @@ -6979,6 +6979,10 @@ on this implementation detail.
> >
> > @hook TARGET_OPTION_VALID_ATTRIBUTE_P
> >
> > +@hook TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P
> > +
> > +@hook TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> > +
> > @hook TARGET_OPTION_SAVE
> >
> > @hook TARGET_OPTION_RESTORE
> > diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
> > index a2ed048d7dd28ec470953fcd8a0dc86817e4b7dc..3db57c2b13d612a37240d9dcf58ad21b2286633c 100644
> > --- a/gcc/multiple_target.cc
> > +++ b/gcc/multiple_target.cc
> > @@ -66,10 +66,6 @@ create_dispatcher_calls (struct cgraph_node *node)
> > {
> > ipa_ref *ref;
> >
> > - if (!DECL_FUNCTION_VERSIONED (node->decl)
> > - || !is_function_default_version (node->decl))
> > - return;
> > -
> > if (!targetm.has_ifunc_p ())
> > {
> > error_at (DECL_SOURCE_LOCATION (node->decl),
> > @@ -377,6 +373,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> > return false;
> > }
> >
> > + const char *new_attr_name = targetm.target_option.expanded_clones_attribute;
> > cgraph_function_version_info *decl1_v = NULL;
> > cgraph_function_version_info *decl2_v = NULL;
> > cgraph_function_version_info *before = NULL;
> > @@ -392,7 +389,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> > char *attr = attrs[i];
> >
> > /* Create new target clone. */
> > - tree attributes = make_attribute ("target", attr,
> > + tree attributes = make_attribute (new_attr_name, attr,
> > DECL_ATTRIBUTES (node->decl));
> >
> > char *suffix = XNEWVEC (char, strlen (attr) + 1);
> > @@ -430,7 +427,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> > XDELETEVEC (attr_str);
> >
> > /* Setting new attribute to initial function. */
> > - tree attributes = make_attribute ("target", "default",
> > + tree attributes = make_attribute (new_attr_name, "default",
> > DECL_ATTRIBUTES (node->decl));
> > DECL_ATTRIBUTES (node->decl) = attributes;
> > node->local = false;
> > diff --git a/gcc/target.def b/gcc/target.def
> > index cda6c51e5167f85625168c7c26b777d6c8ccad82..39acea04db01ebaf918910b7dd73d397de6a84ec 100644
> > --- a/gcc/target.def
> > +++ b/gcc/target.def
> > @@ -6492,6 +6492,31 @@ the function declaration to hold a pointer to a target-specific\n\
> > bool, (tree fndecl, tree name, tree args, int flags),
> > default_target_option_valid_attribute_p)
> >
> > +/* Function to validate the attribute((target_version(...))) strings. If
> > + the option is validated, the hook should also fill in
> > + DECL_FUNCTION_SPECIFIC_TARGET in the function decl node. */
> > +DEFHOOK
> > +(valid_version_attribute_p,
> > + "This hook is called to parse @code{attribute(target_version(\"...\"))},\n\
> > +which allows setting target-specific options on individual function versions.\n\
> > +These function-specific options may differ\n\
> > +from the options specified on the command line. The hook should return\n\
> > +@code{true} if the options are valid.\n\
> > +\n\
> > +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in\n\
> > +the function declaration to hold a pointer to a target-specific\n\
> > +@code{struct cl_target_option} structure.",
> > + bool, (tree fndecl, tree name, tree args, int flags),
> > + default_target_option_valid_version_attribute_p)
> > +
> > +/* Attribute to be used when expanding clones for functions with
> > + target_clones attribute. */
> > +DEFHOOKPOD
> > +(expanded_clones_attribute,
> > + "Contains the name of the attribute used for the version description string\n\
> > +when expanding clones for a function with the target_clones attribute.",
> > + const char *, "target")
> > +
> > /* Function to save any extra target state in the target options structure. */
> > DEFHOOK
> > (save,
> > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > index 1a0db8dddd594d9b1fb04ae0d9a66ad6b7a396dc..0efc993d82ef59b581a1df74ee0de71135a28703 100644
> > --- a/gcc/targhooks.h
> > +++ b/gcc/targhooks.h
> > @@ -192,6 +192,7 @@ extern bool default_hard_regno_scratch_ok (unsigned int);
> > extern bool default_mode_dependent_address_p (const_rtx, addr_space_t);
> > extern bool default_new_address_profitable_p (rtx, rtx_insn *, rtx);
> > extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
> > +extern bool default_target_option_valid_version_attribute_p (tree, tree, tree, int);
> > extern bool default_target_option_pragma_parse (tree, tree);
> > extern bool default_target_can_inline_p (tree, tree);
> > extern bool default_update_ipa_fn_target_info (unsigned int &, const gimple *);
> > diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> > index e190369f87a92e6a92372dc348d9374c3a965c0a..7fc7bf455e80c333cced1bac7085210c2b108f8d 100644
> > --- a/gcc/targhooks.cc
> > +++ b/gcc/targhooks.cc
> > @@ -1787,7 +1787,19 @@ default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl),
> > int ARG_UNUSED (flags))
> > {
> > warning (OPT_Wattributes,
> > - "target attribute is not supported on this machine");
> > + "%<target%> attribute is not supported on this machine");
> > +
> > + return false;
> > +}
> > +
> > +bool
> > +default_target_option_valid_version_attribute_p (tree ARG_UNUSED (fndecl),
> > + tree ARG_UNUSED (name),
> > + tree ARG_UNUSED (args),
> > + int ARG_UNUSED (flags))
> > +{
> > + warning (OPT_Wattributes,
> > + "%<target_version%> attribute is not supported on this machine");
> >
> > return false;
> > }
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index 0b72663e6a1a94406127f6253460f498b7a3ea9c..ebd89ce79566c350eaaab210c0dca3cc1ac2048e 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -3438,8 +3438,8 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree);
> > (FUNCTION_DECL_CHECK (NODE)->function_decl.function_specific_optimization)
> >
> > /* In FUNCTION_DECL, this is set if this function has other versions generated
> > - using "target" attributes. The default version is the one which does not
> > - have any "target" attribute set. */
> > + to support different architecture feature sets, e.g. using "target" or
> > + "target_version" attributes. */
> > #define DECL_FUNCTION_VERSIONED(NODE)\
> > (FUNCTION_DECL_CHECK (NODE)->function_decl.versioned_function)
> >
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> This patch adds support for the "target_version" attribute to the middle
> end and the C++ frontend, which will be used to implement function
> multiversioning in the aarch64 backend.
>
> Note that C++ is currently the only frontend which supports
> multiversioning using the "target" attribute, whereas the
> "target_clones" attribute is additionally supported in C, D and Ada.
> Support for the target_version attribute will be extended to C at a
> later date.
>
> Targets that currently use the "target" attribute for function
> multiversioning (i.e. i386 and rs6000) are not affected by this patch.
>
>
> I could have implemented the target hooks slightly differently, by reusing the
> valid_attribute_p hook and adding attribute name checks to each backend
> implementation (c.f. the aarch64 implementation in patch 2/3). Would this be
> preferable?
Having as much as possible in target-independent code seems better
to me FWIW. On that basis:
>
> Otherwise, is this ok for master?
>
>
> gcc/c-family/ChangeLog:
>
> * c-attribs.cc (handle_target_version_attribute): New.
> (c_common_attribute_table): Add target_version.
> (handle_target_clones_attribute): Add conflict with
> target_version attribute.
>
> gcc/ChangeLog:
>
> * attribs.cc (is_function_default_version): Update comment to
> specify incompatibility with target_version attributes.
> * cgraphclones.cc (cgraph_node::create_version_clone_with_body):
> Call valid_version_attribute_p for target_version attributes.
> * target.def (valid_version_attribute_p): New hook.
> (expanded_clones_attribute): New hook.
> * doc/tm.texi.in: Add new hooks.
> * doc/tm.texi: Regenerate.
> * multiple_target.cc (create_dispatcher_calls): Remove redundant
> is_function_default_version check.
> (expand_target_clones): Use target hook for attribute name.
> * targhooks.cc (default_target_option_valid_version_attribute_p):
> New.
> * targhooks.h (default_target_option_valid_version_attribute_p):
> New.
> * tree.h (DECL_FUNCTION_VERSIONED): Update comment to include
> target_version attributes.
>
> gcc/cp/ChangeLog:
>
> * decl2.cc (check_classfn): Update comment to include
> target_version attributes.
>
>
> diff --git a/gcc/attribs.cc b/gcc/attribs.cc
> index b1300018d1e8ed8e02ded1ea721dc192a6d32a49..a3c4a81e8582ea4fd06b9518bf51fad7c998ddd6 100644
> --- a/gcc/attribs.cc
> +++ b/gcc/attribs.cc
> @@ -1233,8 +1233,9 @@ make_dispatcher_decl (const tree decl)
> return func_decl;
> }
>
> -/* Returns true if decl is multi-versioned and DECL is the default function,
> - that is it is not tagged with target specific optimization. */
> +/* Returns true if DECL is multi-versioned using the target attribute, and this
> + is the default version. This function can only be used for targets that do
> + not support the "target_version" attribute. */
>
> bool
> is_function_default_version (const tree decl)
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 072cfb69147bd6b314459c0bd48a0c1fb92d3e4d..1a224c036277d51ab4dc0d33a403177bd226e48a 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -148,6 +148,7 @@ static tree handle_alloc_align_attribute (tree *, tree, tree, int, bool *);
> static tree handle_assume_aligned_attribute (tree *, tree, tree, int, bool *);
> static tree handle_assume_attribute (tree *, tree, tree, int, bool *);
> static tree handle_target_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_target_version_attribute (tree *, tree, tree, int, bool *);
> static tree handle_target_clones_attribute (tree *, tree, tree, int, bool *);
> static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
> static tree ignore_attribute (tree *, tree, tree, int, bool *);
> @@ -480,6 +481,8 @@ const struct attribute_spec c_common_attribute_table[] =
> handle_error_attribute, NULL },
> { "target", 1, -1, true, false, false, false,
> handle_target_attribute, NULL },
> + { "target_version", 1, -1, true, false, false, false,
> + handle_target_version_attribute, NULL },
> { "target_clones", 1, -1, true, false, false, false,
> handle_target_clones_attribute, NULL },
> { "optimize", 1, -1, true, false, false, false,
> @@ -5569,6 +5572,45 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
> return NULL_TREE;
> }
>
> +/* Handle a "target_version" attribute. */
> +
> +static tree
> +handle_target_version_attribute (tree *node, tree name, tree args, int flags,
> + bool *no_add_attrs)
> +{
> + /* Ensure we have a function type. */
> + if (TREE_CODE (*node) != FUNCTION_DECL)
> + {
> + warning (OPT_Wattributes, "%qE attribute ignored", name);
> + *no_add_attrs = true;
> + }
> + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (*node)))
> + {
> + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> + "with %qs attribute", name, "target_clones");
> + *no_add_attrs = true;
> + }
> + else if (!targetm.target_option.valid_version_attribute_p (*node, name, args,
> + flags))
> + *no_add_attrs = true;
> +
> + /* Check that there's no empty string in values of the attribute. */
> + for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
> + {
> + tree value = TREE_VALUE (t);
> + if (TREE_CODE (value) == STRING_CST
> + && TREE_STRING_LENGTH (value) == 1
> + && TREE_STRING_POINTER (value)[0] == '\0')
> + {
> + warning (OPT_Wattributes,
> + "empty string in attribute %<target_version%>");
> + *no_add_attrs = true;
> + }
> + }
would it make sense to do the empty string test first, and only pass
the vetted arguments to the target hook? Also, a Google search suggests
that there aren't any pre-existing, conflicting uses of "target_version"
that take multiple arguments. So could this code check that there
is exactly one argument (by changing 1, -1 to 1, 1 in the spec above)
and then require it to be a nonempty string? It could then pass the
string itself to the target hook (probably as a const char *).
(FWIW, it doesn't look like the Clang documentation has kept the door
open to multiple arguments.)
I wonder if we could use attribute_spec::exclusions to describe the
mutual exclusion with "target_clones". It doesn't look like the
existing code does, though, so maybe not.
I couldn't see anything that forbids a combination of "target" and
"target_version". Should that combination be allowed? In some ways
it makes conceptual sense, since using "target" is like changing the
command-line options. But I suppose we'd then need to diagnose conflicts
and deal with ordering issues. So perhaps "target" should be made
mutually exclusive as well.
Thanks,
Richard
> +
> + return NULL_TREE;
> +}
> +
> /* Handle a "target_clones" attribute. */
>
> static tree
> @@ -5601,6 +5643,12 @@ handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
> "with %qs attribute", name, "target");
> *no_add_attrs = true;
> }
> + else if (lookup_attribute ("target_version", DECL_ATTRIBUTES (*node)))
> + {
> + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> + "with %qs attribute", name, "target_version");
> + *no_add_attrs = true;
> + }
> else if (get_target_clone_attr_len (args) == -1)
> {
> warning (OPT_Wattributes,
> diff --git a/gcc/cgraphclones.cc b/gcc/cgraphclones.cc
> index 29d28ef895a73a223695cbb86aafbc845bbe7688..8af6b23d8c0306920e0fdcb3559ef047a16689f4 100644
> --- a/gcc/cgraphclones.cc
> +++ b/gcc/cgraphclones.cc
> @@ -78,6 +78,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-eh.h"
> #include "tree-cfg.h"
> #include "tree-inline.h"
> +#include "attribs.h"
> #include "dumpfile.h"
> #include "gimple-pretty-print.h"
> #include "alloc-pool.h"
> @@ -1048,7 +1049,17 @@ cgraph_node::create_version_clone_with_body
> location_t saved_loc = input_location;
> tree v = TREE_VALUE (target_attributes);
> input_location = DECL_SOURCE_LOCATION (new_decl);
> - bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
> + bool r;
> + tree name_id = get_attribute_name (target_attributes);
> + const char* name_str = IDENTIFIER_POINTER (name_id);
> + if (strcmp (name_str, "target") == 0)
> + r = targetm.target_option.valid_attribute_p (new_decl, name_id, v, 1);
> + else if (strcmp (name_str, "target_version") == 0)
> + r = targetm.target_option.valid_version_attribute_p (new_decl, name_id,
> + v, 1);
> + else
> + gcc_assert(false);
> +
> input_location = saved_loc;
> if (!r)
> return NULL;
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 9594be4092c3c00fddc9d4c6da5931ea3b7e8792..ec78d5a5440bedd360ac8e5bc44e164da3dab410 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -829,8 +829,8 @@ check_classfn (tree ctype, tree function, tree template_parms)
> tree c2 = get_constraints (fndecl);
>
> /* While finding a match, same types and params are not enough
> - if the function is versioned. Also check version ("target")
> - attributes. */
> + if the function is versioned. Also check for different target
> + specific attributes. */
> if (same_type_p (TREE_TYPE (TREE_TYPE (function)),
> TREE_TYPE (TREE_TYPE (fndecl)))
> && compparms (p1, p2)
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 33e7ffc8af5a9d48430145ef9b8e7924613b7dd7..97b3e9c31631a943d95f1cf7739716a574afcfb7 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -10507,6 +10507,23 @@ the function declaration to hold a pointer to a target-specific
> @code{struct cl_target_option} structure.
> @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P (tree @var{fndecl}, tree @var{name}, tree @var{args}, int @var{flags})
> +This hook is called to parse @code{attribute(target_version("..."))},
> +which allows setting target-specific options on individual function versions.
> +These function-specific options may differ
> +from the options specified on the command line. The hook should return
> +@code{true} if the options are valid.
> +
> +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in
> +the function declaration to hold a pointer to a target-specific
> +@code{struct cl_target_option} structure.
> +@end deftypefn
> +
> +@deftypevr {Target Hook} {const char *} TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> +Contains the name of the attribute used for the version description string
> +when expanding clones for a function with the target_clones attribute.
> +@end deftypevr
> +
> @deftypefn {Target Hook} void TARGET_OPTION_SAVE (struct cl_target_option *@var{ptr}, struct gcc_options *@var{opts}, struct gcc_options *@var{opts_set})
> This hook is called to save any additional target-specific information
> in the @code{struct cl_target_option} structure for function-specific
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index c98b2447e28aa17996b1cbf8af7ed02d70db54f2..56fa3de6bba06bc0ac124bb3a41324be1997e209 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -6979,6 +6979,10 @@ on this implementation detail.
>
> @hook TARGET_OPTION_VALID_ATTRIBUTE_P
>
> +@hook TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P
> +
> +@hook TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> +
> @hook TARGET_OPTION_SAVE
>
> @hook TARGET_OPTION_RESTORE
> diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
> index a2ed048d7dd28ec470953fcd8a0dc86817e4b7dc..3db57c2b13d612a37240d9dcf58ad21b2286633c 100644
> --- a/gcc/multiple_target.cc
> +++ b/gcc/multiple_target.cc
> @@ -66,10 +66,6 @@ create_dispatcher_calls (struct cgraph_node *node)
> {
> ipa_ref *ref;
>
> - if (!DECL_FUNCTION_VERSIONED (node->decl)
> - || !is_function_default_version (node->decl))
> - return;
> -
> if (!targetm.has_ifunc_p ())
> {
> error_at (DECL_SOURCE_LOCATION (node->decl),
> @@ -377,6 +373,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> return false;
> }
>
> + const char *new_attr_name = targetm.target_option.expanded_clones_attribute;
> cgraph_function_version_info *decl1_v = NULL;
> cgraph_function_version_info *decl2_v = NULL;
> cgraph_function_version_info *before = NULL;
> @@ -392,7 +389,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> char *attr = attrs[i];
>
> /* Create new target clone. */
> - tree attributes = make_attribute ("target", attr,
> + tree attributes = make_attribute (new_attr_name, attr,
> DECL_ATTRIBUTES (node->decl));
>
> char *suffix = XNEWVEC (char, strlen (attr) + 1);
> @@ -430,7 +427,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> XDELETEVEC (attr_str);
>
> /* Setting new attribute to initial function. */
> - tree attributes = make_attribute ("target", "default",
> + tree attributes = make_attribute (new_attr_name, "default",
> DECL_ATTRIBUTES (node->decl));
> DECL_ATTRIBUTES (node->decl) = attributes;
> node->local = false;
> diff --git a/gcc/target.def b/gcc/target.def
> index cda6c51e5167f85625168c7c26b777d6c8ccad82..39acea04db01ebaf918910b7dd73d397de6a84ec 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -6492,6 +6492,31 @@ the function declaration to hold a pointer to a target-specific\n\
> bool, (tree fndecl, tree name, tree args, int flags),
> default_target_option_valid_attribute_p)
>
> +/* Function to validate the attribute((target_version(...))) strings. If
> + the option is validated, the hook should also fill in
> + DECL_FUNCTION_SPECIFIC_TARGET in the function decl node. */
> +DEFHOOK
> +(valid_version_attribute_p,
> + "This hook is called to parse @code{attribute(target_version(\"...\"))},\n\
> +which allows setting target-specific options on individual function versions.\n\
> +These function-specific options may differ\n\
> +from the options specified on the command line. The hook should return\n\
> +@code{true} if the options are valid.\n\
> +\n\
> +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in\n\
> +the function declaration to hold a pointer to a target-specific\n\
> +@code{struct cl_target_option} structure.",
> + bool, (tree fndecl, tree name, tree args, int flags),
> + default_target_option_valid_version_attribute_p)
> +
> +/* Attribute to be used when expanding clones for functions with
> + target_clones attribute. */
> +DEFHOOKPOD
> +(expanded_clones_attribute,
> + "Contains the name of the attribute used for the version description string\n\
> +when expanding clones for a function with the target_clones attribute.",
> + const char *, "target")
> +
> /* Function to save any extra target state in the target options structure. */
> DEFHOOK
> (save,
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 1a0db8dddd594d9b1fb04ae0d9a66ad6b7a396dc..0efc993d82ef59b581a1df74ee0de71135a28703 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -192,6 +192,7 @@ extern bool default_hard_regno_scratch_ok (unsigned int);
> extern bool default_mode_dependent_address_p (const_rtx, addr_space_t);
> extern bool default_new_address_profitable_p (rtx, rtx_insn *, rtx);
> extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
> +extern bool default_target_option_valid_version_attribute_p (tree, tree, tree, int);
> extern bool default_target_option_pragma_parse (tree, tree);
> extern bool default_target_can_inline_p (tree, tree);
> extern bool default_update_ipa_fn_target_info (unsigned int &, const gimple *);
> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> index e190369f87a92e6a92372dc348d9374c3a965c0a..7fc7bf455e80c333cced1bac7085210c2b108f8d 100644
> --- a/gcc/targhooks.cc
> +++ b/gcc/targhooks.cc
> @@ -1787,7 +1787,19 @@ default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl),
> int ARG_UNUSED (flags))
> {
> warning (OPT_Wattributes,
> - "target attribute is not supported on this machine");
> + "%<target%> attribute is not supported on this machine");
> +
> + return false;
> +}
> +
> +bool
> +default_target_option_valid_version_attribute_p (tree ARG_UNUSED (fndecl),
> + tree ARG_UNUSED (name),
> + tree ARG_UNUSED (args),
> + int ARG_UNUSED (flags))
> +{
> + warning (OPT_Wattributes,
> + "%<target_version%> attribute is not supported on this machine");
>
> return false;
> }
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0b72663e6a1a94406127f6253460f498b7a3ea9c..ebd89ce79566c350eaaab210c0dca3cc1ac2048e 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -3438,8 +3438,8 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree);
> (FUNCTION_DECL_CHECK (NODE)->function_decl.function_specific_optimization)
>
> /* In FUNCTION_DECL, this is set if this function has other versions generated
> - using "target" attributes. The default version is the one which does not
> - have any "target" attribute set. */
> + to support different architecture feature sets, e.g. using "target" or
> + "target_version" attributes. */
> #define DECL_FUNCTION_VERSIONED(NODE)\
> (FUNCTION_DECL_CHECK (NODE)->function_decl.versioned_function)
>
On Thu, Oct 26, 2023 at 07:41:09PM +0100, Richard Sandiford wrote:
> Andrew Carlotti <andrew.carlotti@arm.com> writes:
> > This patch adds support for the "target_version" attribute to the middle
> > end and the C++ frontend, which will be used to implement function
> > multiversioning in the aarch64 backend.
> >
> > Note that C++ is currently the only frontend which supports
> > multiversioning using the "target" attribute, whereas the
> > "target_clones" attribute is additionally supported in C, D and Ada.
> > Support for the target_version attribute will be extended to C at a
> > later date.
> >
> > Targets that currently use the "target" attribute for function
> > multiversioning (i.e. i386 and rs6000) are not affected by this patch.
> >
> >
> > I could have implemented the target hooks slightly differently, by reusing the
> > valid_attribute_p hook and adding attribute name checks to each backend
> > implementation (c.f. the aarch64 implementation in patch 2/3). Would this be
> > preferable?
>
> Having as much as possible in target-independent code seems better
> to me FWIW. On that basis:
>
> >
> > Otherwise, is this ok for master?
> >
> >
> > gcc/c-family/ChangeLog:
> >
> > * c-attribs.cc (handle_target_version_attribute): New.
> > (c_common_attribute_table): Add target_version.
> > (handle_target_clones_attribute): Add conflict with
> > target_version attribute.
> >
> > gcc/ChangeLog:
> >
> > * attribs.cc (is_function_default_version): Update comment to
> > specify incompatibility with target_version attributes.
> > * cgraphclones.cc (cgraph_node::create_version_clone_with_body):
> > Call valid_version_attribute_p for target_version attributes.
> > * target.def (valid_version_attribute_p): New hook.
> > (expanded_clones_attribute): New hook.
> > * doc/tm.texi.in: Add new hooks.
> > * doc/tm.texi: Regenerate.
> > * multiple_target.cc (create_dispatcher_calls): Remove redundant
> > is_function_default_version check.
> > (expand_target_clones): Use target hook for attribute name.
> > * targhooks.cc (default_target_option_valid_version_attribute_p):
> > New.
> > * targhooks.h (default_target_option_valid_version_attribute_p):
> > New.
> > * tree.h (DECL_FUNCTION_VERSIONED): Update comment to include
> > target_version attributes.
> >
> > gcc/cp/ChangeLog:
> >
> > * decl2.cc (check_classfn): Update comment to include
> > target_version attributes.
> >
> >
> > diff --git a/gcc/attribs.cc b/gcc/attribs.cc
> > index b1300018d1e8ed8e02ded1ea721dc192a6d32a49..a3c4a81e8582ea4fd06b9518bf51fad7c998ddd6 100644
> > --- a/gcc/attribs.cc
> > +++ b/gcc/attribs.cc
> > @@ -1233,8 +1233,9 @@ make_dispatcher_decl (const tree decl)
> > return func_decl;
> > }
> >
> > -/* Returns true if decl is multi-versioned and DECL is the default function,
> > - that is it is not tagged with target specific optimization. */
> > +/* Returns true if DECL is multi-versioned using the target attribute, and this
> > + is the default version. This function can only be used for targets that do
> > + not support the "target_version" attribute. */
> >
> > bool
> > is_function_default_version (const tree decl)
> > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> > index 072cfb69147bd6b314459c0bd48a0c1fb92d3e4d..1a224c036277d51ab4dc0d33a403177bd226e48a 100644
> > --- a/gcc/c-family/c-attribs.cc
> > +++ b/gcc/c-family/c-attribs.cc
> > @@ -148,6 +148,7 @@ static tree handle_alloc_align_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_assume_aligned_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_assume_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_target_attribute (tree *, tree, tree, int, bool *);
> > +static tree handle_target_version_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_target_clones_attribute (tree *, tree, tree, int, bool *);
> > static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
> > static tree ignore_attribute (tree *, tree, tree, int, bool *);
> > @@ -480,6 +481,8 @@ const struct attribute_spec c_common_attribute_table[] =
> > handle_error_attribute, NULL },
> > { "target", 1, -1, true, false, false, false,
> > handle_target_attribute, NULL },
> > + { "target_version", 1, -1, true, false, false, false,
> > + handle_target_version_attribute, NULL },
> > { "target_clones", 1, -1, true, false, false, false,
> > handle_target_clones_attribute, NULL },
> > { "optimize", 1, -1, true, false, false, false,
> > @@ -5569,6 +5572,45 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
> > return NULL_TREE;
> > }
> >
> > +/* Handle a "target_version" attribute. */
> > +
> > +static tree
> > +handle_target_version_attribute (tree *node, tree name, tree args, int flags,
> > + bool *no_add_attrs)
> > +{
> > + /* Ensure we have a function type. */
> > + if (TREE_CODE (*node) != FUNCTION_DECL)
> > + {
> > + warning (OPT_Wattributes, "%qE attribute ignored", name);
> > + *no_add_attrs = true;
> > + }
> > + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (*node)))
> > + {
> > + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> > + "with %qs attribute", name, "target_clones");
> > + *no_add_attrs = true;
> > + }
> > + else if (!targetm.target_option.valid_version_attribute_p (*node, name, args,
> > + flags))
> > + *no_add_attrs = true;
> > +
> > + /* Check that there's no empty string in values of the attribute. */
> > + for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
> > + {
> > + tree value = TREE_VALUE (t);
> > + if (TREE_CODE (value) == STRING_CST
> > + && TREE_STRING_LENGTH (value) == 1
> > + && TREE_STRING_POINTER (value)[0] == '\0')
> > + {
> > + warning (OPT_Wattributes,
> > + "empty string in attribute %<target_version%>");
> > + *no_add_attrs = true;
> > + }
> > + }
>
> would it make sense to do the empty string test first, and only pass
> the vetted arguments to the target hook? Also, a Google search suggests
> that there aren't any pre-existing, conflicting uses of "target_version"
> that take multiple arguments. So could this code check that there
> is exactly one argument (by changing 1, -1 to 1, 1 in the spec above)
> and then require it to be a nonempty string? It could then pass the
> string itself to the target hook (probably as a const char *).
>
> (FWIW, it doesn't look like the Clang documentation has kept the door
> open to multiple arguments.)
>
> I wonder if we could use attribute_spec::exclusions to describe the
> mutual exclusion with "target_clones". It doesn't look like the
> existing code does, though, so maybe not.
>
> I couldn't see anything that forbids a combination of "target" and
> "target_version". Should that combination be allowed? In some ways
> it makes conceptual sense, since using "target" is like changing the
> command-line options. But I suppose we'd then need to diagnose conflicts
> and deal with ordering issues. So perhaps "target" should be made
> mutually exclusive as well.
My aarch64 backend pass deals with backend issues by always applying
target_version attribute changes after target_attribute changes. I don't think
there's any additional conflicts to worry about, since adding a target_version
is simply equivalent to enabling extra features in the target string.
A similar thing would work for target_clones, but I didn't initially do that
because it would require making the frontend exclusions target dependant.
However, I think it's just a case of checking the backend hook to see whether
target_clones gets expanded to target attributes or not.
Clang currently disallows combining target attributes with either
target_version or target_clones. However, I think it's worth being able to
combine these attributes. For example, it could be useful to use the target
attribute to select different tuning for an sve target_version.
> Thanks,
> Richard
>
> > +
> > + return NULL_TREE;
> > +}
> > +
> > /* Handle a "target_clones" attribute. */
> >
> > static tree
> > @@ -5601,6 +5643,12 @@ handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
> > "with %qs attribute", name, "target");
> > *no_add_attrs = true;
> > }
> > + else if (lookup_attribute ("target_version", DECL_ATTRIBUTES (*node)))
> > + {
> > + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
> > + "with %qs attribute", name, "target_version");
> > + *no_add_attrs = true;
> > + }
> > else if (get_target_clone_attr_len (args) == -1)
> > {
> > warning (OPT_Wattributes,
> > diff --git a/gcc/cgraphclones.cc b/gcc/cgraphclones.cc
> > index 29d28ef895a73a223695cbb86aafbc845bbe7688..8af6b23d8c0306920e0fdcb3559ef047a16689f4 100644
> > --- a/gcc/cgraphclones.cc
> > +++ b/gcc/cgraphclones.cc
> > @@ -78,6 +78,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "tree-eh.h"
> > #include "tree-cfg.h"
> > #include "tree-inline.h"
> > +#include "attribs.h"
> > #include "dumpfile.h"
> > #include "gimple-pretty-print.h"
> > #include "alloc-pool.h"
> > @@ -1048,7 +1049,17 @@ cgraph_node::create_version_clone_with_body
> > location_t saved_loc = input_location;
> > tree v = TREE_VALUE (target_attributes);
> > input_location = DECL_SOURCE_LOCATION (new_decl);
> > - bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
> > + bool r;
> > + tree name_id = get_attribute_name (target_attributes);
> > + const char* name_str = IDENTIFIER_POINTER (name_id);
> > + if (strcmp (name_str, "target") == 0)
> > + r = targetm.target_option.valid_attribute_p (new_decl, name_id, v, 1);
> > + else if (strcmp (name_str, "target_version") == 0)
> > + r = targetm.target_option.valid_version_attribute_p (new_decl, name_id,
> > + v, 1);
> > + else
> > + gcc_assert(false);
> > +
> > input_location = saved_loc;
> > if (!r)
> > return NULL;
> > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> > index 9594be4092c3c00fddc9d4c6da5931ea3b7e8792..ec78d5a5440bedd360ac8e5bc44e164da3dab410 100644
> > --- a/gcc/cp/decl2.cc
> > +++ b/gcc/cp/decl2.cc
> > @@ -829,8 +829,8 @@ check_classfn (tree ctype, tree function, tree template_parms)
> > tree c2 = get_constraints (fndecl);
> >
> > /* While finding a match, same types and params are not enough
> > - if the function is versioned. Also check version ("target")
> > - attributes. */
> > + if the function is versioned. Also check for different target
> > + specific attributes. */
> > if (same_type_p (TREE_TYPE (TREE_TYPE (function)),
> > TREE_TYPE (TREE_TYPE (fndecl)))
> > && compparms (p1, p2)
> > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > index 33e7ffc8af5a9d48430145ef9b8e7924613b7dd7..97b3e9c31631a943d95f1cf7739716a574afcfb7 100644
> > --- a/gcc/doc/tm.texi
> > +++ b/gcc/doc/tm.texi
> > @@ -10507,6 +10507,23 @@ the function declaration to hold a pointer to a target-specific
> > @code{struct cl_target_option} structure.
> > @end deftypefn
> >
> > +@deftypefn {Target Hook} bool TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P (tree @var{fndecl}, tree @var{name}, tree @var{args}, int @var{flags})
> > +This hook is called to parse @code{attribute(target_version("..."))},
> > +which allows setting target-specific options on individual function versions.
> > +These function-specific options may differ
> > +from the options specified on the command line. The hook should return
> > +@code{true} if the options are valid.
> > +
> > +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in
> > +the function declaration to hold a pointer to a target-specific
> > +@code{struct cl_target_option} structure.
> > +@end deftypefn
> > +
> > +@deftypevr {Target Hook} {const char *} TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> > +Contains the name of the attribute used for the version description string
> > +when expanding clones for a function with the target_clones attribute.
> > +@end deftypevr
> > +
> > @deftypefn {Target Hook} void TARGET_OPTION_SAVE (struct cl_target_option *@var{ptr}, struct gcc_options *@var{opts}, struct gcc_options *@var{opts_set})
> > This hook is called to save any additional target-specific information
> > in the @code{struct cl_target_option} structure for function-specific
> > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > index c98b2447e28aa17996b1cbf8af7ed02d70db54f2..56fa3de6bba06bc0ac124bb3a41324be1997e209 100644
> > --- a/gcc/doc/tm.texi.in
> > +++ b/gcc/doc/tm.texi.in
> > @@ -6979,6 +6979,10 @@ on this implementation detail.
> >
> > @hook TARGET_OPTION_VALID_ATTRIBUTE_P
> >
> > +@hook TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P
> > +
> > +@hook TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
> > +
> > @hook TARGET_OPTION_SAVE
> >
> > @hook TARGET_OPTION_RESTORE
> > diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
> > index a2ed048d7dd28ec470953fcd8a0dc86817e4b7dc..3db57c2b13d612a37240d9dcf58ad21b2286633c 100644
> > --- a/gcc/multiple_target.cc
> > +++ b/gcc/multiple_target.cc
> > @@ -66,10 +66,6 @@ create_dispatcher_calls (struct cgraph_node *node)
> > {
> > ipa_ref *ref;
> >
> > - if (!DECL_FUNCTION_VERSIONED (node->decl)
> > - || !is_function_default_version (node->decl))
> > - return;
> > -
> > if (!targetm.has_ifunc_p ())
> > {
> > error_at (DECL_SOURCE_LOCATION (node->decl),
> > @@ -377,6 +373,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> > return false;
> > }
> >
> > + const char *new_attr_name = targetm.target_option.expanded_clones_attribute;
> > cgraph_function_version_info *decl1_v = NULL;
> > cgraph_function_version_info *decl2_v = NULL;
> > cgraph_function_version_info *before = NULL;
> > @@ -392,7 +389,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> > char *attr = attrs[i];
> >
> > /* Create new target clone. */
> > - tree attributes = make_attribute ("target", attr,
> > + tree attributes = make_attribute (new_attr_name, attr,
> > DECL_ATTRIBUTES (node->decl));
> >
> > char *suffix = XNEWVEC (char, strlen (attr) + 1);
> > @@ -430,7 +427,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
> > XDELETEVEC (attr_str);
> >
> > /* Setting new attribute to initial function. */
> > - tree attributes = make_attribute ("target", "default",
> > + tree attributes = make_attribute (new_attr_name, "default",
> > DECL_ATTRIBUTES (node->decl));
> > DECL_ATTRIBUTES (node->decl) = attributes;
> > node->local = false;
> > diff --git a/gcc/target.def b/gcc/target.def
> > index cda6c51e5167f85625168c7c26b777d6c8ccad82..39acea04db01ebaf918910b7dd73d397de6a84ec 100644
> > --- a/gcc/target.def
> > +++ b/gcc/target.def
> > @@ -6492,6 +6492,31 @@ the function declaration to hold a pointer to a target-specific\n\
> > bool, (tree fndecl, tree name, tree args, int flags),
> > default_target_option_valid_attribute_p)
> >
> > +/* Function to validate the attribute((target_version(...))) strings. If
> > + the option is validated, the hook should also fill in
> > + DECL_FUNCTION_SPECIFIC_TARGET in the function decl node. */
> > +DEFHOOK
> > +(valid_version_attribute_p,
> > + "This hook is called to parse @code{attribute(target_version(\"...\"))},\n\
> > +which allows setting target-specific options on individual function versions.\n\
> > +These function-specific options may differ\n\
> > +from the options specified on the command line. The hook should return\n\
> > +@code{true} if the options are valid.\n\
> > +\n\
> > +The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in\n\
> > +the function declaration to hold a pointer to a target-specific\n\
> > +@code{struct cl_target_option} structure.",
> > + bool, (tree fndecl, tree name, tree args, int flags),
> > + default_target_option_valid_version_attribute_p)
> > +
> > +/* Attribute to be used when expanding clones for functions with
> > + target_clones attribute. */
> > +DEFHOOKPOD
> > +(expanded_clones_attribute,
> > + "Contains the name of the attribute used for the version description string\n\
> > +when expanding clones for a function with the target_clones attribute.",
> > + const char *, "target")
> > +
> > /* Function to save any extra target state in the target options structure. */
> > DEFHOOK
> > (save,
> > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > index 1a0db8dddd594d9b1fb04ae0d9a66ad6b7a396dc..0efc993d82ef59b581a1df74ee0de71135a28703 100644
> > --- a/gcc/targhooks.h
> > +++ b/gcc/targhooks.h
> > @@ -192,6 +192,7 @@ extern bool default_hard_regno_scratch_ok (unsigned int);
> > extern bool default_mode_dependent_address_p (const_rtx, addr_space_t);
> > extern bool default_new_address_profitable_p (rtx, rtx_insn *, rtx);
> > extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
> > +extern bool default_target_option_valid_version_attribute_p (tree, tree, tree, int);
> > extern bool default_target_option_pragma_parse (tree, tree);
> > extern bool default_target_can_inline_p (tree, tree);
> > extern bool default_update_ipa_fn_target_info (unsigned int &, const gimple *);
> > diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> > index e190369f87a92e6a92372dc348d9374c3a965c0a..7fc7bf455e80c333cced1bac7085210c2b108f8d 100644
> > --- a/gcc/targhooks.cc
> > +++ b/gcc/targhooks.cc
> > @@ -1787,7 +1787,19 @@ default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl),
> > int ARG_UNUSED (flags))
> > {
> > warning (OPT_Wattributes,
> > - "target attribute is not supported on this machine");
> > + "%<target%> attribute is not supported on this machine");
> > +
> > + return false;
> > +}
> > +
> > +bool
> > +default_target_option_valid_version_attribute_p (tree ARG_UNUSED (fndecl),
> > + tree ARG_UNUSED (name),
> > + tree ARG_UNUSED (args),
> > + int ARG_UNUSED (flags))
> > +{
> > + warning (OPT_Wattributes,
> > + "%<target_version%> attribute is not supported on this machine");
> >
> > return false;
> > }
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index 0b72663e6a1a94406127f6253460f498b7a3ea9c..ebd89ce79566c350eaaab210c0dca3cc1ac2048e 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -3438,8 +3438,8 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree);
> > (FUNCTION_DECL_CHECK (NODE)->function_decl.function_specific_optimization)
> >
> > /* In FUNCTION_DECL, this is set if this function has other versions generated
> > - using "target" attributes. The default version is the one which does not
> > - have any "target" attribute set. */
> > + to support different architecture feature sets, e.g. using "target" or
> > + "target_version" attributes. */
> > #define DECL_FUNCTION_VERSIONED(NODE)\
> > (FUNCTION_DECL_CHECK (NODE)->function_decl.versioned_function)
> >
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> On Thu, Oct 26, 2023 at 07:41:09PM +0100, Richard Sandiford wrote:
>> Andrew Carlotti <andrew.carlotti@arm.com> writes:
>> > This patch adds support for the "target_version" attribute to the middle
>> > end and the C++ frontend, which will be used to implement function
>> > multiversioning in the aarch64 backend.
>> >
>> > Note that C++ is currently the only frontend which supports
>> > multiversioning using the "target" attribute, whereas the
>> > "target_clones" attribute is additionally supported in C, D and Ada.
>> > Support for the target_version attribute will be extended to C at a
>> > later date.
>> >
>> > Targets that currently use the "target" attribute for function
>> > multiversioning (i.e. i386 and rs6000) are not affected by this patch.
>> >
>> >
>> > I could have implemented the target hooks slightly differently, by reusing the
>> > valid_attribute_p hook and adding attribute name checks to each backend
>> > implementation (c.f. the aarch64 implementation in patch 2/3). Would this be
>> > preferable?
>>
>> Having as much as possible in target-independent code seems better
>> to me FWIW. On that basis:
>>
>> >
>> > Otherwise, is this ok for master?
>> >
>> >
>> > gcc/c-family/ChangeLog:
>> >
>> > * c-attribs.cc (handle_target_version_attribute): New.
>> > (c_common_attribute_table): Add target_version.
>> > (handle_target_clones_attribute): Add conflict with
>> > target_version attribute.
>> >
>> > gcc/ChangeLog:
>> >
>> > * attribs.cc (is_function_default_version): Update comment to
>> > specify incompatibility with target_version attributes.
>> > * cgraphclones.cc (cgraph_node::create_version_clone_with_body):
>> > Call valid_version_attribute_p for target_version attributes.
>> > * target.def (valid_version_attribute_p): New hook.
>> > (expanded_clones_attribute): New hook.
>> > * doc/tm.texi.in: Add new hooks.
>> > * doc/tm.texi: Regenerate.
>> > * multiple_target.cc (create_dispatcher_calls): Remove redundant
>> > is_function_default_version check.
>> > (expand_target_clones): Use target hook for attribute name.
>> > * targhooks.cc (default_target_option_valid_version_attribute_p):
>> > New.
>> > * targhooks.h (default_target_option_valid_version_attribute_p):
>> > New.
>> > * tree.h (DECL_FUNCTION_VERSIONED): Update comment to include
>> > target_version attributes.
>> >
>> > gcc/cp/ChangeLog:
>> >
>> > * decl2.cc (check_classfn): Update comment to include
>> > target_version attributes.
>> >
>> >
>> > diff --git a/gcc/attribs.cc b/gcc/attribs.cc
>> > index b1300018d1e8ed8e02ded1ea721dc192a6d32a49..a3c4a81e8582ea4fd06b9518bf51fad7c998ddd6 100644
>> > --- a/gcc/attribs.cc
>> > +++ b/gcc/attribs.cc
>> > @@ -1233,8 +1233,9 @@ make_dispatcher_decl (const tree decl)
>> > return func_decl;
>> > }
>> >
>> > -/* Returns true if decl is multi-versioned and DECL is the default function,
>> > - that is it is not tagged with target specific optimization. */
>> > +/* Returns true if DECL is multi-versioned using the target attribute, and this
>> > + is the default version. This function can only be used for targets that do
>> > + not support the "target_version" attribute. */
>> >
>> > bool
>> > is_function_default_version (const tree decl)
>> > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
>> > index 072cfb69147bd6b314459c0bd48a0c1fb92d3e4d..1a224c036277d51ab4dc0d33a403177bd226e48a 100644
>> > --- a/gcc/c-family/c-attribs.cc
>> > +++ b/gcc/c-family/c-attribs.cc
>> > @@ -148,6 +148,7 @@ static tree handle_alloc_align_attribute (tree *, tree, tree, int, bool *);
>> > static tree handle_assume_aligned_attribute (tree *, tree, tree, int, bool *);
>> > static tree handle_assume_attribute (tree *, tree, tree, int, bool *);
>> > static tree handle_target_attribute (tree *, tree, tree, int, bool *);
>> > +static tree handle_target_version_attribute (tree *, tree, tree, int, bool *);
>> > static tree handle_target_clones_attribute (tree *, tree, tree, int, bool *);
>> > static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
>> > static tree ignore_attribute (tree *, tree, tree, int, bool *);
>> > @@ -480,6 +481,8 @@ const struct attribute_spec c_common_attribute_table[] =
>> > handle_error_attribute, NULL },
>> > { "target", 1, -1, true, false, false, false,
>> > handle_target_attribute, NULL },
>> > + { "target_version", 1, -1, true, false, false, false,
>> > + handle_target_version_attribute, NULL },
>> > { "target_clones", 1, -1, true, false, false, false,
>> > handle_target_clones_attribute, NULL },
>> > { "optimize", 1, -1, true, false, false, false,
>> > @@ -5569,6 +5572,45 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
>> > return NULL_TREE;
>> > }
>> >
>> > +/* Handle a "target_version" attribute. */
>> > +
>> > +static tree
>> > +handle_target_version_attribute (tree *node, tree name, tree args, int flags,
>> > + bool *no_add_attrs)
>> > +{
>> > + /* Ensure we have a function type. */
>> > + if (TREE_CODE (*node) != FUNCTION_DECL)
>> > + {
>> > + warning (OPT_Wattributes, "%qE attribute ignored", name);
>> > + *no_add_attrs = true;
>> > + }
>> > + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (*node)))
>> > + {
>> > + warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
>> > + "with %qs attribute", name, "target_clones");
>> > + *no_add_attrs = true;
>> > + }
>> > + else if (!targetm.target_option.valid_version_attribute_p (*node, name, args,
>> > + flags))
>> > + *no_add_attrs = true;
>> > +
>> > + /* Check that there's no empty string in values of the attribute. */
>> > + for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
>> > + {
>> > + tree value = TREE_VALUE (t);
>> > + if (TREE_CODE (value) == STRING_CST
>> > + && TREE_STRING_LENGTH (value) == 1
>> > + && TREE_STRING_POINTER (value)[0] == '\0')
>> > + {
>> > + warning (OPT_Wattributes,
>> > + "empty string in attribute %<target_version%>");
>> > + *no_add_attrs = true;
>> > + }
>> > + }
>>
>> would it make sense to do the empty string test first, and only pass
>> the vetted arguments to the target hook? Also, a Google search suggests
>> that there aren't any pre-existing, conflicting uses of "target_version"
>> that take multiple arguments. So could this code check that there
>> is exactly one argument (by changing 1, -1 to 1, 1 in the spec above)
>> and then require it to be a nonempty string? It could then pass the
>> string itself to the target hook (probably as a const char *).
>>
>> (FWIW, it doesn't look like the Clang documentation has kept the door
>> open to multiple arguments.)
>>
>> I wonder if we could use attribute_spec::exclusions to describe the
>> mutual exclusion with "target_clones". It doesn't look like the
>> existing code does, though, so maybe not.
>>
>> I couldn't see anything that forbids a combination of "target" and
>> "target_version". Should that combination be allowed? In some ways
>> it makes conceptual sense, since using "target" is like changing the
>> command-line options. But I suppose we'd then need to diagnose conflicts
>> and deal with ordering issues. So perhaps "target" should be made
>> mutually exclusive as well.
>
> My aarch64 backend pass deals with backend issues by always applying
> target_version attribute changes after target_attribute changes. I don't think
> there's any additional conflicts to worry about, since adding a target_version
> is simply equivalent to enabling extra features in the target string.
>
> A similar thing would work for target_clones, but I didn't initially do that
> because it would require making the frontend exclusions target dependant.
> However, I think it's just a case of checking the backend hook to see whether
> target_clones gets expanded to target attributes or not.
>
> Clang currently disallows combining target attributes with either
> target_version or target_clones. However, I think it's worth being able to
> combine these attributes. For example, it could be useful to use the target
> attribute to select different tuning for an sve target_version.
Ah, ok, thanks for the explanation. That approach sounds good to me.
Richard
@@ -1233,8 +1233,9 @@ make_dispatcher_decl (const tree decl)
return func_decl;
}
-/* Returns true if decl is multi-versioned and DECL is the default function,
- that is it is not tagged with target specific optimization. */
+/* Returns true if DECL is multi-versioned using the target attribute, and this
+ is the default version. This function can only be used for targets that do
+ not support the "target_version" attribute. */
bool
is_function_default_version (const tree decl)
@@ -148,6 +148,7 @@ static tree handle_alloc_align_attribute (tree *, tree, tree, int, bool *);
static tree handle_assume_aligned_attribute (tree *, tree, tree, int, bool *);
static tree handle_assume_attribute (tree *, tree, tree, int, bool *);
static tree handle_target_attribute (tree *, tree, tree, int, bool *);
+static tree handle_target_version_attribute (tree *, tree, tree, int, bool *);
static tree handle_target_clones_attribute (tree *, tree, tree, int, bool *);
static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
static tree ignore_attribute (tree *, tree, tree, int, bool *);
@@ -480,6 +481,8 @@ const struct attribute_spec c_common_attribute_table[] =
handle_error_attribute, NULL },
{ "target", 1, -1, true, false, false, false,
handle_target_attribute, NULL },
+ { "target_version", 1, -1, true, false, false, false,
+ handle_target_version_attribute, NULL },
{ "target_clones", 1, -1, true, false, false, false,
handle_target_clones_attribute, NULL },
{ "optimize", 1, -1, true, false, false, false,
@@ -5569,6 +5572,45 @@ handle_target_attribute (tree *node, tree name, tree args, int flags,
return NULL_TREE;
}
+/* Handle a "target_version" attribute. */
+
+static tree
+handle_target_version_attribute (tree *node, tree name, tree args, int flags,
+ bool *no_add_attrs)
+{
+ /* Ensure we have a function type. */
+ if (TREE_CODE (*node) != FUNCTION_DECL)
+ {
+ warning (OPT_Wattributes, "%qE attribute ignored", name);
+ *no_add_attrs = true;
+ }
+ else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (*node)))
+ {
+ warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
+ "with %qs attribute", name, "target_clones");
+ *no_add_attrs = true;
+ }
+ else if (!targetm.target_option.valid_version_attribute_p (*node, name, args,
+ flags))
+ *no_add_attrs = true;
+
+ /* Check that there's no empty string in values of the attribute. */
+ for (tree t = args; t != NULL_TREE; t = TREE_CHAIN (t))
+ {
+ tree value = TREE_VALUE (t);
+ if (TREE_CODE (value) == STRING_CST
+ && TREE_STRING_LENGTH (value) == 1
+ && TREE_STRING_POINTER (value)[0] == '\0')
+ {
+ warning (OPT_Wattributes,
+ "empty string in attribute %<target_version%>");
+ *no_add_attrs = true;
+ }
+ }
+
+ return NULL_TREE;
+}
+
/* Handle a "target_clones" attribute. */
static tree
@@ -5601,6 +5643,12 @@ handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
"with %qs attribute", name, "target");
*no_add_attrs = true;
}
+ else if (lookup_attribute ("target_version", DECL_ATTRIBUTES (*node)))
+ {
+ warning (OPT_Wattributes, "%qE attribute ignored due to conflict "
+ "with %qs attribute", name, "target_version");
+ *no_add_attrs = true;
+ }
else if (get_target_clone_attr_len (args) == -1)
{
warning (OPT_Wattributes,
@@ -78,6 +78,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-eh.h"
#include "tree-cfg.h"
#include "tree-inline.h"
+#include "attribs.h"
#include "dumpfile.h"
#include "gimple-pretty-print.h"
#include "alloc-pool.h"
@@ -1048,7 +1049,17 @@ cgraph_node::create_version_clone_with_body
location_t saved_loc = input_location;
tree v = TREE_VALUE (target_attributes);
input_location = DECL_SOURCE_LOCATION (new_decl);
- bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
+ bool r;
+ tree name_id = get_attribute_name (target_attributes);
+ const char* name_str = IDENTIFIER_POINTER (name_id);
+ if (strcmp (name_str, "target") == 0)
+ r = targetm.target_option.valid_attribute_p (new_decl, name_id, v, 1);
+ else if (strcmp (name_str, "target_version") == 0)
+ r = targetm.target_option.valid_version_attribute_p (new_decl, name_id,
+ v, 1);
+ else
+ gcc_assert(false);
+
input_location = saved_loc;
if (!r)
return NULL;
@@ -829,8 +829,8 @@ check_classfn (tree ctype, tree function, tree template_parms)
tree c2 = get_constraints (fndecl);
/* While finding a match, same types and params are not enough
- if the function is versioned. Also check version ("target")
- attributes. */
+ if the function is versioned. Also check for different target
+ specific attributes. */
if (same_type_p (TREE_TYPE (TREE_TYPE (function)),
TREE_TYPE (TREE_TYPE (fndecl)))
&& compparms (p1, p2)
@@ -10507,6 +10507,23 @@ the function declaration to hold a pointer to a target-specific
@code{struct cl_target_option} structure.
@end deftypefn
+@deftypefn {Target Hook} bool TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P (tree @var{fndecl}, tree @var{name}, tree @var{args}, int @var{flags})
+This hook is called to parse @code{attribute(target_version("..."))},
+which allows setting target-specific options on individual function versions.
+These function-specific options may differ
+from the options specified on the command line. The hook should return
+@code{true} if the options are valid.
+
+The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in
+the function declaration to hold a pointer to a target-specific
+@code{struct cl_target_option} structure.
+@end deftypefn
+
+@deftypevr {Target Hook} {const char *} TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
+Contains the name of the attribute used for the version description string
+when expanding clones for a function with the target_clones attribute.
+@end deftypevr
+
@deftypefn {Target Hook} void TARGET_OPTION_SAVE (struct cl_target_option *@var{ptr}, struct gcc_options *@var{opts}, struct gcc_options *@var{opts_set})
This hook is called to save any additional target-specific information
in the @code{struct cl_target_option} structure for function-specific
@@ -6979,6 +6979,10 @@ on this implementation detail.
@hook TARGET_OPTION_VALID_ATTRIBUTE_P
+@hook TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P
+
+@hook TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE
+
@hook TARGET_OPTION_SAVE
@hook TARGET_OPTION_RESTORE
@@ -66,10 +66,6 @@ create_dispatcher_calls (struct cgraph_node *node)
{
ipa_ref *ref;
- if (!DECL_FUNCTION_VERSIONED (node->decl)
- || !is_function_default_version (node->decl))
- return;
-
if (!targetm.has_ifunc_p ())
{
error_at (DECL_SOURCE_LOCATION (node->decl),
@@ -377,6 +373,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
return false;
}
+ const char *new_attr_name = targetm.target_option.expanded_clones_attribute;
cgraph_function_version_info *decl1_v = NULL;
cgraph_function_version_info *decl2_v = NULL;
cgraph_function_version_info *before = NULL;
@@ -392,7 +389,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
char *attr = attrs[i];
/* Create new target clone. */
- tree attributes = make_attribute ("target", attr,
+ tree attributes = make_attribute (new_attr_name, attr,
DECL_ATTRIBUTES (node->decl));
char *suffix = XNEWVEC (char, strlen (attr) + 1);
@@ -430,7 +427,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
XDELETEVEC (attr_str);
/* Setting new attribute to initial function. */
- tree attributes = make_attribute ("target", "default",
+ tree attributes = make_attribute (new_attr_name, "default",
DECL_ATTRIBUTES (node->decl));
DECL_ATTRIBUTES (node->decl) = attributes;
node->local = false;
@@ -6492,6 +6492,31 @@ the function declaration to hold a pointer to a target-specific\n\
bool, (tree fndecl, tree name, tree args, int flags),
default_target_option_valid_attribute_p)
+/* Function to validate the attribute((target_version(...))) strings. If
+ the option is validated, the hook should also fill in
+ DECL_FUNCTION_SPECIFIC_TARGET in the function decl node. */
+DEFHOOK
+(valid_version_attribute_p,
+ "This hook is called to parse @code{attribute(target_version(\"...\"))},\n\
+which allows setting target-specific options on individual function versions.\n\
+These function-specific options may differ\n\
+from the options specified on the command line. The hook should return\n\
+@code{true} if the options are valid.\n\
+\n\
+The hook should set the @code{DECL_FUNCTION_SPECIFIC_TARGET} field in\n\
+the function declaration to hold a pointer to a target-specific\n\
+@code{struct cl_target_option} structure.",
+ bool, (tree fndecl, tree name, tree args, int flags),
+ default_target_option_valid_version_attribute_p)
+
+/* Attribute to be used when expanding clones for functions with
+ target_clones attribute. */
+DEFHOOKPOD
+(expanded_clones_attribute,
+ "Contains the name of the attribute used for the version description string\n\
+when expanding clones for a function with the target_clones attribute.",
+ const char *, "target")
+
/* Function to save any extra target state in the target options structure. */
DEFHOOK
(save,
@@ -192,6 +192,7 @@ extern bool default_hard_regno_scratch_ok (unsigned int);
extern bool default_mode_dependent_address_p (const_rtx, addr_space_t);
extern bool default_new_address_profitable_p (rtx, rtx_insn *, rtx);
extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
+extern bool default_target_option_valid_version_attribute_p (tree, tree, tree, int);
extern bool default_target_option_pragma_parse (tree, tree);
extern bool default_target_can_inline_p (tree, tree);
extern bool default_update_ipa_fn_target_info (unsigned int &, const gimple *);
@@ -1787,7 +1787,19 @@ default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl),
int ARG_UNUSED (flags))
{
warning (OPT_Wattributes,
- "target attribute is not supported on this machine");
+ "%<target%> attribute is not supported on this machine");
+
+ return false;
+}
+
+bool
+default_target_option_valid_version_attribute_p (tree ARG_UNUSED (fndecl),
+ tree ARG_UNUSED (name),
+ tree ARG_UNUSED (args),
+ int ARG_UNUSED (flags))
+{
+ warning (OPT_Wattributes,
+ "%<target_version%> attribute is not supported on this machine");
return false;
}
@@ -3438,8 +3438,8 @@ extern vec<tree, va_gc> **decl_debug_args_insert (tree);
(FUNCTION_DECL_CHECK (NODE)->function_decl.function_specific_optimization)
/* In FUNCTION_DECL, this is set if this function has other versions generated
- using "target" attributes. The default version is the one which does not
- have any "target" attribute set. */
+ to support different architecture feature sets, e.g. using "target" or
+ "target_version" attributes. */
#define DECL_FUNCTION_VERSIONED(NODE)\
(FUNCTION_DECL_CHECK (NODE)->function_decl.versioned_function)