Re: [PATCH 1/2] IPA symver: allow multiple symvers for a definition
Hi. There's one obvious fix that I've just noticed. I'm going to install it. Martin >From a5d075d595d35cd5d6607bbd9c8b2eb7707ca920 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Wed, 26 Aug 2020 13:18:14 +0200 Subject: [PATCH] symver: fix attribute matching. gcc/ChangeLog: * cgraphunit.c (process_symver_attribute): Match only symver TREE_PURPOSE. --- gcc/cgraphunit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index fa3aec79a48..26d3995a0c0 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -727,6 +727,9 @@ process_symver_attribute (symtab_node *n) .symver foo, bar@V1 .symver foo, baz@V2 */ + const char *purpose = IDENTIFIER_POINTER (TREE_PURPOSE (value)); + if (strcmp (purpose, "symver") != 0) + continue; tree symver = get_identifier_with_length (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (value))), -- 2.28.0
Re: [PATCH 1/2] IPA symver: allow multiple symvers for a definition
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 000..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;
+ }
+ }]
+}
Re: [PATCH 1/2] IPA symver: allow multiple symvers for a definition
>
> 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 000..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;
> + }
> + }]
> +}
[PATCH 1/2] IPA symver: allow multiple symvers for a definition
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
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),
+ "% 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),
- "% 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) ==
