This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Make target_clones resolver fn static.
On 1/20/20 3:52 PM, Richard Biener wrote:
On Mon, Jan 20, 2020 at 3:46 PM H.J. Lu <hjl.tools@gmail.com> wrote:
On Mon, Jan 20, 2020 at 6:41 AM Alexander Monakov <amonakov@ispras.ru> wrote:
On Mon, 20 Jan 2020, H.J. Lu wrote:
Bare IFUNC's don't seem to have this restriction. Why do we want to
constrain target clones this way?
foo's resolver acts as foo. It should have the same visibility as foo.
What do you mean by that? From the implementation standpoint, there's
two symbols of different type with the same value. There's no problem
allowing one of them have local binding and the other have global binding.
Is there something special about target clones that doesn't come into
play with ifuncs?
I stand corrected. Resolver should be static and it shouldn't be weak.
Reading the patch again and looking up more context it seems that the resolver
is already static we just mangle it extra when the original function
is public (?)
If so the patch looks quite obvious to me if we use some character not valid
in indetifiers in C but valid in assembly for the resolver decl.
Hello.
I'm sending updated version of the patch where I'm adding a run-time test
for 2 static symbols (with the same name) and it works fine. Moreover
I'm newly setting TREE_PUBLIC (ifunc_alias_decl) = 0 which seems to
work properly.
I tested both x86_64-linux-gnu and ppc64le-linux-gnu.
Martin
Richard.
--
H.J.
>From a3faaced989869867671ceadd89b56fabde225ff Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 16 Jan 2020 10:38:41 +0100
Subject: [PATCH] Make target_clones resolver fn static.
gcc/ChangeLog:
2020-01-17 Martin Liska <mliska@suse.cz>
PR target/93274
* config/i386/i386-features.c (make_resolver_func):
Align the code with ppc64 target implementaion.
We do not need to have gnu_indirect_function
as a global function. Drop TREE_PUBLIC on
ifunc symbol.
* config/rs6000/rs6000.c (make_resolver_func): Drop
TREE_PUBLIC on ifunc symbol.
gcc/testsuite/ChangeLog:
2020-01-17 Martin Liska <mliska@suse.cz>
PR target/93274
* gcc.target/i386/pr81213.c: Adjust to not expect
a global unique name.
* gcc.target/i386/pr81213-2.c: New test.
---
gcc/config/i386/i386-features.c | 23 ++++++++---------------
gcc/config/rs6000/rs6000.c | 12 ++++++++++++
gcc/testsuite/gcc.target/i386/pr81213-2.c | 11 +++++++++++
gcc/testsuite/gcc.target/i386/pr81213.c | 11 +++++++----
4 files changed, 38 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr81213-2.c
diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index e580b26b995..07eca286544 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -2738,26 +2738,17 @@ make_resolver_func (const tree default_decl,
const tree ifunc_alias_decl,
basic_block *empty_bb)
{
- char *resolver_name;
- tree decl, type, decl_name, t;
+ tree decl, type, t;
- /* IFUNC's have to be globally visible. So, if the default_decl is
- not, then the name of the IFUNC should be made unique. */
- if (TREE_PUBLIC (default_decl) == 0)
- {
- char *ifunc_name = make_unique_name (default_decl, "ifunc", true);
- symtab->change_decl_assembler_name (ifunc_alias_decl,
- get_identifier (ifunc_name));
- XDELETEVEC (ifunc_name);
- }
-
- resolver_name = make_unique_name (default_decl, "resolver", false);
+ /* Make the resolver function static. The resolver function returns
+ void *. */
+ tree decl_name = clone_function_name (default_decl, "resolver");
+ const char *resolver_name = IDENTIFIER_POINTER (decl_name);
/* The resolver function should return a (void *). */
type = build_function_type_list (ptr_type_node, NULL_TREE);
decl = build_fn_decl (resolver_name, type);
- decl_name = get_identifier (resolver_name);
SET_DECL_ASSEMBLER_NAME (decl, decl_name);
DECL_NAME (decl) = decl_name;
@@ -2784,6 +2775,9 @@ make_resolver_func (const tree default_decl,
DECL_COMDAT (decl) = 1;
make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
}
+ else
+ TREE_PUBLIC (ifunc_alias_decl) = 0;
+
/* Build result decl and add to function_decl. */
t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node);
DECL_CONTEXT (t) = decl;
@@ -2809,7 +2803,6 @@ make_resolver_func (const tree default_decl,
/* Create the alias for dispatch to resolver here. */
cgraph_node::create_same_body_alias (ifunc_alias_decl, decl);
- XDELETEVEC (resolver_name);
return decl;
}
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 127927d9583..97b8ebe4d80 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -23858,6 +23858,18 @@ make_resolver_func (const tree default_decl,
DECL_INITIAL (decl) = make_node (BLOCK);
DECL_STATIC_CONSTRUCTOR (decl) = 0;
+ if (DECL_COMDAT_GROUP (default_decl)
+ || TREE_PUBLIC (default_decl))
+ {
+ /* In this case, each translation unit with a call to this
+ versioned function will put out a resolver. Ensure it
+ is comdat to keep just one copy. */
+ DECL_COMDAT (decl) = 1;
+ make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
+ }
+ else
+ TREE_PUBLIC (dispatch_decl) = 0;
+
/* Build result decl and add to function_decl. */
tree t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node);
DECL_CONTEXT (t) = decl;
diff --git a/gcc/testsuite/gcc.target/i386/pr81213-2.c b/gcc/testsuite/gcc.target/i386/pr81213-2.c
new file mode 100644
index 00000000000..a80622cb184
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr81213-2.c
@@ -0,0 +1,11 @@
+__attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))
+static int
+foo ()
+{
+ return 2;
+}
+
+int bar()
+{
+ return foo();
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr81213.c b/gcc/testsuite/gcc.target/i386/pr81213.c
index 13e15d5fef0..334838631d0 100644
--- a/gcc/testsuite/gcc.target/i386/pr81213.c
+++ b/gcc/testsuite/gcc.target/i386/pr81213.c
@@ -1,6 +1,9 @@
/* PR ipa/81214. */
-/* { dg-do compile } */
+/* { dg-do run } */
/* { dg-require-ifunc "" } */
+/* { dg-additional-sources "pr81213-2.c" } */
+
+int bar();
__attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))
static int
@@ -11,9 +14,9 @@ foo ()
int main()
{
- return foo();
+ return foo() + bar();
}
-/* { dg-final { scan-assembler "\t.globl\tfoo\\..*\\.ifunc" } } */
+/* { dg-final { scan-assembler "\t.globl\tfoo" } } */
/* { dg-final { scan-assembler "foo.resolver:" } } */
-/* { dg-final { scan-assembler "foo\\..*\\.ifunc, @gnu_indirect_function" } } */
+/* { dg-final { scan-assembler "foo\\, @gnu_indirect_function" } } */
--
2.24.1