| Message ID | 20241015061843.3005429-1-chenyangyu@isrc.iscas.ac.cn |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce TARGET_FMV_ATTR_SEPARATOR | expand |
On Tue, Oct 15, 2024 at 02:18:43PM +0800, Yangyu Chen 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_FMV_ATTR_SEPARATOR to separate different > clones. This is only for the target_clones attribute, so how about calling it TARGET_CLONES_ATTR_SEPARATOR instead (or pluralised - see below)? > 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_FMV_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_FMV_ATTR_SEPARATOR): Define new macro. > * multiple_target.cc (get_attr_str): Use > TARGET_FMV_ATTR_SEPARATOR to separate attributes. > (separate_attrs): Likewise. > * config/riscv/riscv.h (TARGET_FMV_ATTR_SEPARATOR): Define > TARGET_FMV_ATTR_SEPARATOR for RISC-V. > --- > gcc/config/riscv/riscv.h | 5 +++++ > gcc/defaults.h | 4 ++++ > gcc/multiple_target.cc | 19 ++++++++++++------- > 3 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h > index ca1b8329cdc..858cab72a4c 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_FMV_ATTR_SEPARATOR '#' > + > #endif /* ! GCC_RISCV_H */ > diff --git a/gcc/defaults.h b/gcc/defaults.h > index ac2d25852ab..f451efcb33e 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_FMV_ATTR_SEPARATOR > +#define TARGET_FMV_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..5a056b44571 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_FMV_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_FMV_ATTR_SEPARATOR); > + p; > + p = strchr (p + 1, TARGET_FMV_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_FMV_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_FMV_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; > + char separator_str[] = {TARGET_FMV_ATTR_SEPARATOR, '\0'}; How about defining the macro as a string (and appending an S to the name - e.g. TARGET_CLONES_ATTR_SEPARATORS)? > - 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_FMV_ATTR_SEPARATOR. */ > tree attr_target = lookup_attribute ("target_clones", > DECL_ATTRIBUTES (node->decl)); > /* No targets specified. */ > -- > 2.45.2 >
> On Oct 15, 2024, at 20:11, Andrew Carlotti <andrew.carlotti@arm.com> wrote: > > On Tue, Oct 15, 2024 at 02:18:43PM +0800, Yangyu Chen 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_FMV_ATTR_SEPARATOR to separate different >> clones. > > This is only for the target_clones attribute, so how about calling it > TARGET_CLONES_ATTR_SEPARATOR instead (or pluralised - see below)? > Sound like a good idea. I choose to use TARGET_CLONES_ATTR_SEPARATOR in the next revision. Link: https://patchwork.sourceware.org/project/gcc/patch/20241015181607.3689413-1-chenyangyu@isrc.iscas.ac.cn/ >> 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_FMV_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_FMV_ATTR_SEPARATOR): Define new macro. >> * multiple_target.cc (get_attr_str): Use >> TARGET_FMV_ATTR_SEPARATOR to separate attributes. >> (separate_attrs): Likewise. >> * config/riscv/riscv.h (TARGET_FMV_ATTR_SEPARATOR): Define >> TARGET_FMV_ATTR_SEPARATOR for RISC-V. >> --- >> gcc/config/riscv/riscv.h | 5 +++++ >> gcc/defaults.h | 4 ++++ >> gcc/multiple_target.cc | 19 ++++++++++++------- >> 3 files changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h >> index ca1b8329cdc..858cab72a4c 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_FMV_ATTR_SEPARATOR '#' >> + >> #endif /* ! GCC_RISCV_H */ >> diff --git a/gcc/defaults.h b/gcc/defaults.h >> index ac2d25852ab..f451efcb33e 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_FMV_ATTR_SEPARATOR >> +#define TARGET_FMV_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..5a056b44571 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_FMV_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_FMV_ATTR_SEPARATOR); >> + p; >> + p = strchr (p + 1, TARGET_FMV_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_FMV_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_FMV_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; >> + char separator_str[] = {TARGET_FMV_ATTR_SEPARATOR, '\0'}; > > How about defining the macro as a string (and appending an S to the name - e.g. > TARGET_CLONES_ATTR_SEPARATORS)? > I didn't find a clean way in C to build a char array based on a single char. I searched some codes in GCC, and I found a similar use case for DIR_SEPARATOR which is: static const char xxxx_str[] = { xxxx, 0 }; So, I edit this to: static const char separator_str[] = { TARGET_CLONES_ATTR_SEPARATOR, 0 }; I think this way is better. Thanks, Yangyu Chen >> - 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_FMV_ATTR_SEPARATOR. */ >> tree attr_target = lookup_attribute ("target_clones", >> DECL_ATTRIBUTES (node->decl)); >> /* No targets specified. */ >> -- >> 2.45.2
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index ca1b8329cdc..858cab72a4c 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_FMV_ATTR_SEPARATOR '#' + #endif /* ! GCC_RISCV_H */ diff --git a/gcc/defaults.h b/gcc/defaults.h index ac2d25852ab..f451efcb33e 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_FMV_ATTR_SEPARATOR +#define TARGET_FMV_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..5a056b44571 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_FMV_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_FMV_ATTR_SEPARATOR); + p; + p = strchr (p + 1, TARGET_FMV_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_FMV_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_FMV_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; + char separator_str[] = {TARGET_FMV_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_FMV_ATTR_SEPARATOR. */ tree attr_target = lookup_attribute ("target_clones", DECL_ATTRIBUTES (node->decl)); /* No targets specified. */
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_FMV_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_FMV_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_FMV_ATTR_SEPARATOR): Define new macro. * multiple_target.cc (get_attr_str): Use TARGET_FMV_ATTR_SEPARATOR to separate attributes. (separate_attrs): Likewise. * config/riscv/riscv.h (TARGET_FMV_ATTR_SEPARATOR): Define TARGET_FMV_ATTR_SEPARATOR for RISC-V. --- gcc/config/riscv/riscv.h | 5 +++++ gcc/defaults.h | 4 ++++ gcc/multiple_target.cc | 19 ++++++++++++------- 3 files changed, 21 insertions(+), 7 deletions(-)