Hi Jeff: Do you mind giving an LGTM/OK for this patch? it's outside RISC-V folder so I think we need you to help :P
And this patch looks OK to me already :) On Thu, Oct 24, 2024 at 3:11 PM Yangyu Chen <c...@cyyself.name> wrote: > > Some architectures may use ',' in the attribute string, but it is not > used as the separator for different targets. To avoid conflict, we > introduce a new macro TARGET_CLONES_ATTR_SEPARATOR to separate different > clones. > > As an example, according to RISC-V C-API Specification [1], RISC-V allows > ',' in the attribute string in the "arch=" option to specify one more > ISA extensions in the same target function, which conflict with the > default separator to separate different clones. This patch introduces > TARGET_CLONES_ATTR_SEPARATOR for RISC-V and choose '#' as the separator, > since '#' is not allowed in the target_clones option string. > > [1] > https://github.com/riscv-non-isa/riscv-c-api-doc/blob/c6c5d6d9cf96b342293315a5dff3d25e96ef8191/src/c-api.adoc#__attribute__targetattr-string > > gcc/ChangeLog: > > * defaults.h (TARGET_CLONES_ATTR_SEPARATOR): Define new macro. > * multiple_target.cc (get_attr_str): Use > TARGET_CLONES_ATTR_SEPARATOR to separate attributes. > (separate_attrs): Likewise. > * attribs.cc (attr_strcmp): Likewise. > (sorted_attr_string): Likewise. > * tree.cc (get_target_clone_attr_len): Likewise. > * config/riscv/riscv.h (TARGET_CLONES_ATTR_SEPARATOR): Define > TARGET_CLONES_ATTR_SEPARATOR for RISC-V. > --- > gcc/attribs.cc | 17 ++++++++++------- > gcc/config/riscv/riscv.h | 5 +++++ > gcc/defaults.h | 4 ++++ > gcc/multiple_target.cc | 19 ++++++++++++------- > gcc/tree.cc | 4 +++- > 5 files changed, 34 insertions(+), 15 deletions(-) > > diff --git a/gcc/attribs.cc b/gcc/attribs.cc > index 0be7b83b264..ab46cb51425 100644 > --- a/gcc/attribs.cc > +++ b/gcc/attribs.cc > @@ -1102,9 +1102,10 @@ attr_strcmp (const void *v1, const void *v2) > } > > /* ARGLIST is the argument to target attribute. This function tokenizes > - the comma separated arguments, sorts them and returns a string which > - is a unique identifier for the comma separated arguments. It also > - replaces non-identifier characters "=,-" with "_". */ > + the TARGET_CLONES_ATTR_SEPARATOR separated arguments, sorts them and > + returns a string which is a unique identifier for the > + TARGET_CLONES_ATTR_SEPARATOR separated arguments. It also replaces > + non-identifier characters "=,-" with "_". */ > > char * > sorted_attr_string (tree arglist) > @@ -1116,6 +1117,7 @@ sorted_attr_string (tree arglist) > char *attr = NULL; > unsigned int argnum = 1; > unsigned int i; > + static const char separator_str[] = { TARGET_CLONES_ATTR_SEPARATOR, 0 }; > > for (arg = arglist; arg; arg = TREE_CHAIN (arg)) > { > @@ -1125,7 +1127,7 @@ sorted_attr_string (tree arglist) > if (arg != arglist) > argnum++; > for (i = 0; i < strlen (str); i++) > - if (str[i] == ',') > + if (str[i] == TARGET_CLONES_ATTR_SEPARATOR) > argnum++; > } > > @@ -1136,7 +1138,8 @@ sorted_attr_string (tree arglist) > const char *str = TREE_STRING_POINTER (TREE_VALUE (arg)); > size_t len = strlen (str); > memcpy (attr_str + str_len_sum, str, len); > - attr_str[str_len_sum + len] = TREE_CHAIN (arg) ? ',' : '\0'; > + attr_str[str_len_sum + len] > + = TREE_CHAIN (arg) ? TARGET_CLONES_ATTR_SEPARATOR : '\0'; > str_len_sum += len + 1; > } > > @@ -1151,12 +1154,12 @@ sorted_attr_string (tree arglist) > args = XNEWVEC (char *, argnum); > > i = 0; > - attr = strtok (attr_str, ","); > + attr = strtok (attr_str, separator_str); > while (attr != NULL) > { > args[i] = attr; > i++; > - attr = strtok (NULL, ","); > + attr = strtok (NULL, separator_str); > } > > qsort (args, argnum, sizeof (char *), attr_strcmp); > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h > index ca1b8329cdc..2ff9c1024f3 100644 > --- a/gcc/config/riscv/riscv.h > +++ b/gcc/config/riscv/riscv.h > @@ -1298,4 +1298,9 @@ extern void riscv_remove_unneeded_save_restore_calls > (void); > STACK_BOUNDARY / BITS_PER_UNIT) \ > : (crtl->outgoing_args_size + STACK_POINTER_OFFSET)) > > +/* According to the RISC-V C API, the arch string may contains ','. To avoid > + the conflict with the default separator, we choose '#' as the separator > for > + the target attribute. */ > +#define TARGET_CLONES_ATTR_SEPARATOR '#' > + > #endif /* ! GCC_RISCV_H */ > diff --git a/gcc/defaults.h b/gcc/defaults.h > index ac2d25852ab..918e3ec2f24 100644 > --- a/gcc/defaults.h > +++ b/gcc/defaults.h > @@ -874,6 +874,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. > If not, see > #define TARGET_HAS_FMV_TARGET_ATTRIBUTE 1 > #endif > > +/* Select a attribute separator for function multiversioning. */ > +#ifndef TARGET_CLONES_ATTR_SEPARATOR > +#define TARGET_CLONES_ATTR_SEPARATOR ',' > +#endif > > /* Select a format to encode pointers in exception handling data. We > prefer those that result in fewer dynamic relocations. Assume no > diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc > index 1fdd279da04..c1e358dfc1e 100644 > --- a/gcc/multiple_target.cc > +++ b/gcc/multiple_target.cc > @@ -180,7 +180,7 @@ create_dispatcher_calls (struct cgraph_node *node) > } > } > > -/* Create string with attributes separated by comma. > +/* Create string with attributes separated by TARGET_CLONES_ATTR_SEPARATOR. > Return number of attributes. */ > > static int > @@ -194,17 +194,21 @@ get_attr_str (tree arglist, char *attr_str) > { > const char *str = TREE_STRING_POINTER (TREE_VALUE (arg)); > size_t len = strlen (str); > - for (const char *p = strchr (str, ','); p; p = strchr (p + 1, ',')) > + for (const char *p = strchr (str, TARGET_CLONES_ATTR_SEPARATOR); > + p; > + p = strchr (p + 1, TARGET_CLONES_ATTR_SEPARATOR)) > argnum++; > memcpy (attr_str + str_len_sum, str, len); > - attr_str[str_len_sum + len] = TREE_CHAIN (arg) ? ',' : '\0'; > + attr_str[str_len_sum + len] > + = TREE_CHAIN (arg) ? TARGET_CLONES_ATTR_SEPARATOR : '\0'; > str_len_sum += len + 1; > argnum++; > } > return argnum; > } > > -/* Return number of attributes separated by comma and put them into ARGS. > +/* Return number of attributes separated by TARGET_CLONES_ATTR_SEPARATOR > + and put them into ARGS. > If there is no DEFAULT attribute return -1. > If there is an empty string in attribute return -2. > If there are multiple DEFAULT attributes return -3. > @@ -215,9 +219,10 @@ separate_attrs (char *attr_str, char **attrs, int > attrnum) > { > int i = 0; > int default_count = 0; > + static const char separator_str[] = { TARGET_CLONES_ATTR_SEPARATOR, 0 }; > > - for (char *attr = strtok (attr_str, ","); > - attr != NULL; attr = strtok (NULL, ",")) > + for (char *attr = strtok (attr_str, separator_str); > + attr != NULL; attr = strtok (NULL, separator_str)) > { > if (strcmp (attr, "default") == 0) > { > @@ -305,7 +310,7 @@ static bool > expand_target_clones (struct cgraph_node *node, bool definition) > { > int i; > - /* Parsing target attributes separated by comma. */ > + /* Parsing target attributes separated by TARGET_CLONES_ATTR_SEPARATOR. */ > tree attr_target = lookup_attribute ("target_clones", > DECL_ATTRIBUTES (node->decl)); > /* No targets specified. */ > diff --git a/gcc/tree.cc b/gcc/tree.cc > index b40f4d31b2f..e9f7f4045a6 100644 > --- a/gcc/tree.cc > +++ b/gcc/tree.cc > @@ -15228,7 +15228,9 @@ get_target_clone_attr_len (tree arglist) > const char *str = TREE_STRING_POINTER (TREE_VALUE (arg)); > size_t len = strlen (str); > str_len_sum += len + 1; > - for (const char *p = strchr (str, ','); p; p = strchr (p + 1, ',')) > + for (const char *p = strchr (str, TARGET_CLONES_ATTR_SEPARATOR); > + p; > + p = strchr (p + 1, TARGET_CLONES_ATTR_SEPARATOR)) > argnum++; > argnum++; > } > -- > 2.45.2 >
