| Message ID | 425a320575d228b9582a2a07576442ed2ec28809.1598348469.git.mliska@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | symver: extend functionality | expand |
> > gcc/ChangeLog: > > * cgraphunit.c (process_symver_attribute): Allow multiple > symver attributes for one symbol. > * doc/extend.texi: Document the change. > > gcc/testsuite/ChangeLog: > > * lib/target-supports-dg.exp: Add dg-require-symver. > * lib/target-supports.exp: Likewise. > * gcc.dg/ipa/symver1.c: New test. > --- > gcc/cgraphunit.c | 143 ++++++++++++----------- > gcc/doc/extend.texi | 10 +- > gcc/testsuite/gcc.dg/ipa/symver1.c | 11 ++ > gcc/testsuite/lib/target-supports-dg.exp | 10 ++ > gcc/testsuite/lib/target-supports.exp | 12 ++ > 5 files changed, 110 insertions(+), 76 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/ipa/symver1.c > > @smallexample > -__attribute__ ((__symver__ ("foo@@VERS_2"))) > -__attribute__ ((alias ("foo_v1"))) > -int symver_foo_v1 (void); > +__attribute__ ((__symver__ ("foo@@VERS_2"), ("foo@@VERS_3"))) > +int symver_foo_v1 (void) So we can still write it as follows: +__attribute__ ((__symver__ ("foo@@VERS_2")))) +__attribute__ ((__symver__ ("foo@@VERS_3")))) +int symver_foo_v1 (void) I think we should support this syntax so the versions can be added separately by macros mixed with other attributs.... Honza > +@{ > +@} > @end smallexample > > This example creates an alias of @code{foo_v1} with symbol name > diff --git a/gcc/testsuite/gcc.dg/ipa/symver1.c b/gcc/testsuite/gcc.dg/ipa/symver1.c > new file mode 100644 > index 00000000000..645de7ea259 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/symver1.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile } */ > + > +__attribute__ ((__symver__ ("foo@VER_2"))) > +__attribute__ ((__symver__ ("foo@VER_3"))) > +int foo() > +{ > + return 2; > +} > + > +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2" } } */ > +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_3" } } */ > diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp > index 5bb99f4e8f9..4a03eaae9ce 100644 > --- a/gcc/testsuite/lib/target-supports-dg.exp > +++ b/gcc/testsuite/lib/target-supports-dg.exp > @@ -665,3 +665,13 @@ if { [info procs saved-dg-process-target] == [list] } { > return [dg-process-target-1 $selector] > } > } > + > +# If this target does not support the "symver" attribute, skip this > +# test. > + > +proc dg-require-symver { args } { > + if { ![ check_symver_available ] } { > + upvar dg-do-what dg-do-what > + set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"] > + } > +} > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index c24330a27ab..f3fc5b80aea 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -10445,3 +10445,15 @@ proc check_effective_target_large_initializer { } { > > return 1 > } > +# Returns 1 if the target toolchain supports extended > +# syntax of .symver directive, 0 otherwise. > + > +proc check_symver_available { } { > + return [check_no_compiler_messages symver_available object { > + int foo(void) { return 0; } > + int main (void) { > + asm volatile (".symver foo,foo@VER_1, local"); > + return 0; > + } > + }] > +}
On 8/25/20 8:48 PM, Jan Hubicka wrote: >> >> gcc/ChangeLog: >> >> * cgraphunit.c (process_symver_attribute): Allow multiple >> symver attributes for one symbol. >> * doc/extend.texi: Document the change. >> >> gcc/testsuite/ChangeLog: >> >> * lib/target-supports-dg.exp: Add dg-require-symver. >> * lib/target-supports.exp: Likewise. >> * gcc.dg/ipa/symver1.c: New test. >> --- >> gcc/cgraphunit.c | 143 ++++++++++++----------- >> gcc/doc/extend.texi | 10 +- >> gcc/testsuite/gcc.dg/ipa/symver1.c | 11 ++ >> gcc/testsuite/lib/target-supports-dg.exp | 10 ++ >> gcc/testsuite/lib/target-supports.exp | 12 ++ >> 5 files changed, 110 insertions(+), 76 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/ipa/symver1.c >> > >> @smallexample >> -__attribute__ ((__symver__ ("foo@@VERS_2"))) >> -__attribute__ ((alias ("foo_v1"))) >> -int symver_foo_v1 (void); >> +__attribute__ ((__symver__ ("foo@@VERS_2"), ("foo@@VERS_3"))) >> +int symver_foo_v1 (void) > So we can still write it as follows: > +__attribute__ ((__symver__ ("foo@@VERS_2")))) > +__attribute__ ((__symver__ ("foo@@VERS_3")))) > +int symver_foo_v1 (void) > > I think we should support this syntax so the versions can be added > separately by macros mixed with other attributs.... Yes, the syntax is supported as well and I'm adding that to documentation. I'm going to push this commit. Thanks for review, Martin > > Honza >> +@{ >> +@} >> @end smallexample >> >> This example creates an alias of @code{foo_v1} with symbol name >> diff --git a/gcc/testsuite/gcc.dg/ipa/symver1.c b/gcc/testsuite/gcc.dg/ipa/symver1.c >> new file mode 100644 >> index 00000000000..645de7ea259 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/ipa/symver1.c >> @@ -0,0 +1,11 @@ >> +/* { dg-do compile } */ >> + >> +__attribute__ ((__symver__ ("foo@VER_2"))) >> +__attribute__ ((__symver__ ("foo@VER_3"))) >> +int foo() >> +{ >> + return 2; >> +} >> + >> +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2" } } */ >> +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_3" } } */ >> diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp >> index 5bb99f4e8f9..4a03eaae9ce 100644 >> --- a/gcc/testsuite/lib/target-supports-dg.exp >> +++ b/gcc/testsuite/lib/target-supports-dg.exp >> @@ -665,3 +665,13 @@ if { [info procs saved-dg-process-target] == [list] } { >> return [dg-process-target-1 $selector] >> } >> } >> + >> +# If this target does not support the "symver" attribute, skip this >> +# test. >> + >> +proc dg-require-symver { args } { >> + if { ![ check_symver_available ] } { >> + upvar dg-do-what dg-do-what >> + set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"] >> + } >> +} >> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >> index c24330a27ab..f3fc5b80aea 100644 >> --- a/gcc/testsuite/lib/target-supports.exp >> +++ b/gcc/testsuite/lib/target-supports.exp >> @@ -10445,3 +10445,15 @@ proc check_effective_target_large_initializer { } { >> >> return 1 >> } >> +# Returns 1 if the target toolchain supports extended >> +# syntax of .symver directive, 0 otherwise. >> + >> +proc check_symver_available { } { >> + return [check_no_compiler_messages symver_available object { >> + int foo(void) { return 0; } >> + int main (void) { >> + asm volatile (".symver foo,foo@VER_1, local"); >> + return 0; >> + } >> + }] >> +} >
Hi. There's one obvious fix that I've just noticed. I'm going to install it. Martin
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 0b1009d0dea..fa3aec79a48 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -720,80 +720,81 @@ process_symver_attribute (symtab_node *n) { tree value = lookup_attribute ("symver", DECL_ATTRIBUTES (n->decl)); - if (!value) - return; - if (lookup_attribute ("symver", TREE_CHAIN (value))) + for (; value != NULL; value = TREE_CHAIN (value)) { - error_at (DECL_SOURCE_LOCATION (n->decl), - "multiple versions for one symbol"); - return; - } - tree symver = get_identifier_with_length - (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (value))), - TREE_STRING_LENGTH (TREE_VALUE (TREE_VALUE (value)))); - symtab_node *def = symtab_node::get_for_asmname (symver); + /* Starting from bintuils 2.35 gas supports: + # Assign foo to bar@V1 and baz@V2. + .symver foo, bar@V1 + .symver foo, baz@V2 + */ + + tree symver = get_identifier_with_length + (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (value))), + TREE_STRING_LENGTH (TREE_VALUE (TREE_VALUE (value)))); + symtab_node *def = symtab_node::get_for_asmname (symver); + + if (def) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "duplicate definition of a symbol version"); + inform (DECL_SOURCE_LOCATION (def->decl), + "same version was previously defined here"); + return; + } + if (!n->definition) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "symbol needs to be defined to have a version"); + return; + } + if (DECL_COMMON (n->decl)) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "common symbol cannot be versioned"); + return; + } + if (DECL_COMDAT (n->decl)) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "comdat symbol cannot be versioned"); + return; + } + if (n->weakref) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "%<weakref%> cannot be versioned"); + return; + } + if (!TREE_PUBLIC (n->decl)) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "versioned symbol must be public"); + return; + } + if (DECL_VISIBILITY (n->decl) != VISIBILITY_DEFAULT) + { + error_at (DECL_SOURCE_LOCATION (n->decl), + "versioned symbol must have default visibility"); + return; + } - if (def) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "duplicate definition of a symbol version"); - inform (DECL_SOURCE_LOCATION (def->decl), - "same version was previously defined here"); - return; - } - if (!n->definition) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "symbol needs to be defined to have a version"); - return; - } - if (DECL_COMMON (n->decl)) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "common symbol cannot be versioned"); - return; - } - if (DECL_COMDAT (n->decl)) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "comdat symbol cannot be versioned"); - return; - } - if (n->weakref) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "%<weakref%> cannot be versioned"); - return; + /* Create new symbol table entry representing the version. */ + tree new_decl = copy_node (n->decl); + + DECL_INITIAL (new_decl) = NULL_TREE; + if (TREE_CODE (new_decl) == FUNCTION_DECL) + DECL_STRUCT_FUNCTION (new_decl) = NULL; + SET_DECL_ASSEMBLER_NAME (new_decl, symver); + TREE_PUBLIC (new_decl) = 1; + DECL_ATTRIBUTES (new_decl) = NULL; + + symtab_node *symver_node = symtab_node::get_create (new_decl); + symver_node->alias = true; + symver_node->definition = true; + symver_node->symver = true; + symver_node->create_reference (n, IPA_REF_ALIAS, NULL); + symver_node->analyzed = true; } - if (!TREE_PUBLIC (n->decl)) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "versioned symbol must be public"); - return; - } - if (DECL_VISIBILITY (n->decl) != VISIBILITY_DEFAULT) - { - error_at (DECL_SOURCE_LOCATION (n->decl), - "versioned symbol must have default visibility"); - return; - } - - /* Create new symbol table entry representing the version. */ - tree new_decl = copy_node (n->decl); - - DECL_INITIAL (new_decl) = NULL_TREE; - if (TREE_CODE (new_decl) == FUNCTION_DECL) - DECL_STRUCT_FUNCTION (new_decl) = NULL; - SET_DECL_ASSEMBLER_NAME (new_decl, symver); - TREE_PUBLIC (new_decl) = 1; - DECL_ATTRIBUTES (new_decl) = NULL; - - symtab_node *symver_node = symtab_node::get_create (new_decl); - symver_node->alias = true; - symver_node->definition = true; - symver_node->symver = true; - symver_node->create_reference (n, IPA_REF_ALIAS, NULL); - symver_node->analyzed = true; } /* Process attributes common for vars and functions. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 2bb9b2f72f5..97ae1ee0843 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -3742,13 +3742,13 @@ foo_v1 (void) Will produce a @code{.symver foo_v1, foo@@VERS_1} directive in the assembler output. -It's an error to define multiple version of a given symbol. In such case -an alias can be used. +One can also define multiple version for a given symbol. @smallexample -__attribute__ ((__symver__ ("foo@@VERS_2"))) -__attribute__ ((alias ("foo_v1"))) -int symver_foo_v1 (void); +__attribute__ ((__symver__ ("foo@@VERS_2"), ("foo@@VERS_3"))) +int symver_foo_v1 (void) +@{ +@} @end smallexample This example creates an alias of @code{foo_v1} with symbol name diff --git a/gcc/testsuite/gcc.dg/ipa/symver1.c b/gcc/testsuite/gcc.dg/ipa/symver1.c new file mode 100644 index 00000000000..645de7ea259 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/symver1.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ + +__attribute__ ((__symver__ ("foo@VER_2"))) +__attribute__ ((__symver__ ("foo@VER_3"))) +int foo() +{ + return 2; +} + +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2" } } */ +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_3" } } */ diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp index 5bb99f4e8f9..4a03eaae9ce 100644 --- a/gcc/testsuite/lib/target-supports-dg.exp +++ b/gcc/testsuite/lib/target-supports-dg.exp @@ -665,3 +665,13 @@ if { [info procs saved-dg-process-target] == [list] } { return [dg-process-target-1 $selector] } } + +# If this target does not support the "symver" attribute, skip this +# test. + +proc dg-require-symver { args } { + if { ![ check_symver_available ] } { + upvar dg-do-what dg-do-what + set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"] + } +} diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index c24330a27ab..f3fc5b80aea 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -10445,3 +10445,15 @@ proc check_effective_target_large_initializer { } { return 1 } +# Returns 1 if the target toolchain supports extended +# syntax of .symver directive, 0 otherwise. + +proc check_symver_available { } { + return [check_no_compiler_messages symver_available object { + int foo(void) { return 0; } + int main (void) { + asm volatile (".symver foo,foo@VER_1, local"); + return 0; + } + }] +}