public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix removal of ifunc (PR ipa/81214).
@ 2017-06-30  8:49 Martin Liška
  2017-06-30 12:38 ` Jan Hubicka
  2017-07-03 13:44 ` Rainer Orth
  0 siblings, 2 replies; 4+ messages in thread
From: Martin Liška @ 2017-06-30  8:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: Michael Meissner

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

Hello.

Following patch fixes the issue where we do not emit ifunc and resolver
for function that are not called in a compilation unit or and not
referenced.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
i386.exp tests work on x86_64-linux-gnu.

Ready to be installed?
Martin

gcc/testsuite/ChangeLog:

2017-06-29  Martin Liska  <mliska@suse.cz>

	PR ipa/81214
	* gcc.target/i386/pr81214.c: New test.

gcc/ChangeLog:

2017-06-29  Martin Liska  <mliska@suse.cz>

	PR ipa/81214
	* multiple_target.c (create_dispatcher_calls): Make ifunc
	also for function that don't have calls or are not referenced.
---
 gcc/multiple_target.c                   | 64 ++++++++++++++++-----------------
 gcc/testsuite/gcc.target/i386/pr81214.c | 14 ++++++++
 2 files changed, 46 insertions(+), 32 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81214.c



[-- Attachment #2: 0001-Fix-removal-of-ifunc-PR-ipa-81214.patch --]
[-- Type: text/x-patch, Size: 3101 bytes --]

diff --git a/gcc/multiple_target.c b/gcc/multiple_target.c
index eddc7d3744b..0d7cc3a2939 100644
--- a/gcc/multiple_target.c
+++ b/gcc/multiple_target.c
@@ -68,6 +68,38 @@ create_dispatcher_calls (struct cgraph_node *node)
       || !is_function_default_version (node->decl))
     return;
 
+  if (!targetm.has_ifunc_p ())
+    {
+      error_at (DECL_SOURCE_LOCATION (node->decl),
+		"the call requires ifunc, which is not"
+		" supported by this target");
+      return;
+    }
+  else if (!targetm.get_function_versions_dispatcher)
+    {
+      error_at (DECL_SOURCE_LOCATION (node->decl),
+		"target does not support function version dispatcher");
+      return;
+    }
+
+  tree idecl = targetm.get_function_versions_dispatcher (node->decl);
+  if (!idecl)
+    {
+      error_at (DECL_SOURCE_LOCATION (node->decl),
+		"default target_clones attribute was not set");
+      return;
+    }
+
+  cgraph_node *inode = cgraph_node::get (idecl);
+  gcc_assert (inode);
+  tree resolver_decl = targetm.generate_version_dispatcher_body (inode);
+
+  /* Update aliases.  */
+  inode->alias = true;
+  inode->alias_target = resolver_decl;
+  if (!inode->analyzed)
+    inode->resolve_alias (cgraph_node::get (resolver_decl));
+
   auto_vec<cgraph_edge *> edges_to_redirect;
   auto_vec<ipa_ref *> references_to_redirect;
 
@@ -80,38 +112,6 @@ create_dispatcher_calls (struct cgraph_node *node)
 
   if (!edges_to_redirect.is_empty () || !references_to_redirect.is_empty ())
     {
-      if (!targetm.has_ifunc_p ())
-	{
-	  error_at (DECL_SOURCE_LOCATION (node->decl),
-		    "the call requires ifunc, which is not"
-		    " supported by this target");
-	  return;
-	}
-      else if (!targetm.get_function_versions_dispatcher)
-	{
-	  error_at (DECL_SOURCE_LOCATION (node->decl),
-		    "target does not support function version dispatcher");
-	  return;
-	}
-
-      tree idecl = targetm.get_function_versions_dispatcher (node->decl);
-      if (!idecl)
-	{
-	  error_at (DECL_SOURCE_LOCATION (node->decl),
-		    "default target_clones attribute was not set");
-	  return;
-	}
-
-      cgraph_node *inode = cgraph_node::get (idecl);
-      gcc_assert (inode);
-      tree resolver_decl = targetm.generate_version_dispatcher_body (inode);
-
-      /* Update aliases.  */
-      inode->alias = true;
-      inode->alias_target = resolver_decl;
-      if (!inode->analyzed)
-	inode->resolve_alias (cgraph_node::get (resolver_decl));
-
       /* Redirect edges.  */
       unsigned i;
       cgraph_edge *e;
diff --git a/gcc/testsuite/gcc.target/i386/pr81214.c b/gcc/testsuite/gcc.target/i386/pr81214.c
new file mode 100644
index 00000000000..2584decdb3c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr81214.c
@@ -0,0 +1,14 @@
+/* PR ipa/81214.  */
+/* { dg-do compile } */
+/* { dg-require-ifunc "" } */
+
+__attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))
+int
+foo ()
+{
+  return -2;
+}
+
+/* { dg-final { scan-assembler "\t.globl\tfoo" } } */
+/* { dg-final { scan-assembler "foo.resolver:" } } */
+/* { dg-final { scan-assembler "foo, @gnu_indirect_function" } } */


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix removal of ifunc (PR ipa/81214).
  2017-06-30  8:49 [PATCH] Fix removal of ifunc (PR ipa/81214) Martin Liška
@ 2017-06-30 12:38 ` Jan Hubicka
  2017-07-03 13:44 ` Rainer Orth
  1 sibling, 0 replies; 4+ messages in thread
From: Jan Hubicka @ 2017-06-30 12:38 UTC (permalink / raw)
  To: Martin Li?ka; +Cc: gcc-patches, Michael Meissner

> Hello.
> 
> Following patch fixes the issue where we do not emit ifunc and resolver
> for function that are not called in a compilation unit or and not
> referenced.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> i386.exp tests work on x86_64-linux-gnu.

OK,
Honza
> 
> Ready to be installed?
> Martin
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-06-29  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/81214
> 	* gcc.target/i386/pr81214.c: New test.
> 
> gcc/ChangeLog:
> 
> 2017-06-29  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/81214
> 	* multiple_target.c (create_dispatcher_calls): Make ifunc
> 	also for function that don't have calls or are not referenced.
> ---
>  gcc/multiple_target.c                   | 64 ++++++++++++++++-----------------
>  gcc/testsuite/gcc.target/i386/pr81214.c | 14 ++++++++
>  2 files changed, 46 insertions(+), 32 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr81214.c
> 
> 

> diff --git a/gcc/multiple_target.c b/gcc/multiple_target.c
> index eddc7d3744b..0d7cc3a2939 100644
> --- a/gcc/multiple_target.c
> +++ b/gcc/multiple_target.c
> @@ -68,6 +68,38 @@ create_dispatcher_calls (struct cgraph_node *node)
>        || !is_function_default_version (node->decl))
>      return;
>  
> +  if (!targetm.has_ifunc_p ())
> +    {
> +      error_at (DECL_SOURCE_LOCATION (node->decl),
> +		"the call requires ifunc, which is not"
> +		" supported by this target");
> +      return;
> +    }
> +  else if (!targetm.get_function_versions_dispatcher)
> +    {
> +      error_at (DECL_SOURCE_LOCATION (node->decl),
> +		"target does not support function version dispatcher");
> +      return;
> +    }
> +
> +  tree idecl = targetm.get_function_versions_dispatcher (node->decl);
> +  if (!idecl)
> +    {
> +      error_at (DECL_SOURCE_LOCATION (node->decl),
> +		"default target_clones attribute was not set");
> +      return;
> +    }
> +
> +  cgraph_node *inode = cgraph_node::get (idecl);
> +  gcc_assert (inode);
> +  tree resolver_decl = targetm.generate_version_dispatcher_body (inode);
> +
> +  /* Update aliases.  */
> +  inode->alias = true;
> +  inode->alias_target = resolver_decl;
> +  if (!inode->analyzed)
> +    inode->resolve_alias (cgraph_node::get (resolver_decl));
> +
>    auto_vec<cgraph_edge *> edges_to_redirect;
>    auto_vec<ipa_ref *> references_to_redirect;
>  
> @@ -80,38 +112,6 @@ create_dispatcher_calls (struct cgraph_node *node)
>  
>    if (!edges_to_redirect.is_empty () || !references_to_redirect.is_empty ())
>      {
> -      if (!targetm.has_ifunc_p ())
> -	{
> -	  error_at (DECL_SOURCE_LOCATION (node->decl),
> -		    "the call requires ifunc, which is not"
> -		    " supported by this target");
> -	  return;
> -	}
> -      else if (!targetm.get_function_versions_dispatcher)
> -	{
> -	  error_at (DECL_SOURCE_LOCATION (node->decl),
> -		    "target does not support function version dispatcher");
> -	  return;
> -	}
> -
> -      tree idecl = targetm.get_function_versions_dispatcher (node->decl);
> -      if (!idecl)
> -	{
> -	  error_at (DECL_SOURCE_LOCATION (node->decl),
> -		    "default target_clones attribute was not set");
> -	  return;
> -	}
> -
> -      cgraph_node *inode = cgraph_node::get (idecl);
> -      gcc_assert (inode);
> -      tree resolver_decl = targetm.generate_version_dispatcher_body (inode);
> -
> -      /* Update aliases.  */
> -      inode->alias = true;
> -      inode->alias_target = resolver_decl;
> -      if (!inode->analyzed)
> -	inode->resolve_alias (cgraph_node::get (resolver_decl));
> -
>        /* Redirect edges.  */
>        unsigned i;
>        cgraph_edge *e;
> diff --git a/gcc/testsuite/gcc.target/i386/pr81214.c b/gcc/testsuite/gcc.target/i386/pr81214.c
> new file mode 100644
> index 00000000000..2584decdb3c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr81214.c
> @@ -0,0 +1,14 @@
> +/* PR ipa/81214.  */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc "" } */
> +
> +__attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))
> +int
> +foo ()
> +{
> +  return -2;
> +}
> +
> +/* { dg-final { scan-assembler "\t.globl\tfoo" } } */
> +/* { dg-final { scan-assembler "foo.resolver:" } } */
> +/* { dg-final { scan-assembler "foo, @gnu_indirect_function" } } */
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix removal of ifunc (PR ipa/81214).
  2017-06-30  8:49 [PATCH] Fix removal of ifunc (PR ipa/81214) Martin Liška
  2017-06-30 12:38 ` Jan Hubicka
@ 2017-07-03 13:44 ` Rainer Orth
  2017-07-04  7:42   ` Martin Liška
  1 sibling, 1 reply; 4+ messages in thread
From: Rainer Orth @ 2017-07-03 13:44 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches, Michael Meissner, Dominique d'Humieres

[-- Attachment #1: Type: text/plain, Size: 2543 bytes --]

Hi Martin,

> Following patch fixes the issue where we do not emit ifunc and resolver
> for function that are not called in a compilation unit or and not
> referenced.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> i386.exp tests work on x86_64-linux-gnu.

your patch caused a testsuite regression on various targets:

FAIL: gcc.target/i386/mvc6.c (test for excess errors)
UNRESOLVED: gcc.target/i386/mvc6.c scan-assembler punpcklbw
UNRESOLVED: gcc.target/i386/mvc6.c scan-assembler vpshufb

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/i386/mvc6.c:8:1: error: the call requires ifunc, which is not supported by this target

I'm seeing it on i386-pc-solaris2.12, Dominique reported it in the PR on
Darwin/x86, and there are also testsuite results on FreeBSD/x86.

Unlike most other __attribute__((target_clones)) tests, this one lacked
{ dg-require-ifunc "" } and didn't need it before.

The following patch fixes this.  Tested with the appropriate runtest
invocation on i386-pc-solaris2.12 and x86_64-pc-linux-gnu, installed on
mainline.

While I was at it, I checked the other testcases with
__attribute__((target_clones)):

g++.dg/ext/mvc1.C		dg-require-ifunc ""
g++.dg/ext/mvc2.C		00 (dg-warning)
g++.dg/ext/mvc3.C		00 (dg-warning)
g++.dg/ext/mvc4.C		dg-require-ifunc ""
gcc.dg/tree-prof/pr66295.c	dg-require-ifunc ""
gcc.target/i386/mvc1.c		dg-require-ifunc ""
gcc.target/i386/mvc2.c		00
gcc.target/i386/mvc3.c		00 (dg-error)
gcc.target/i386/mvc4.c		dg-require-ifunc ""
gcc.target/i386/mvc5.c		dg-require-ifunc ""
gcc.target/i386/mvc6.c		00
gcc.target/i386/mvc7.c		dg-require-ifunc ""
gcc.target/i386/mvc8.c		dg-require-ifunc ""
gcc.target/i386/mvc9.c		dg-require-ifunc ""
gcc.target/i386/pr78419.c	dg-require-ifunc ""
gcc.target/i386/pr80732.c	dg-require-ifunc ""
gcc.target/i386/pr81214.c	dg-require-ifunc ""
gcc.target/powerpc/clone1.c	powerpc*-*-linux* && lp64

Of those without dg-require-ifunc, the powerpc one is (sort of) ok since
it's restricted to Linux, and those with dg-warning/dg-error are too
since the warnings is emitted before the error about missing ifunc
support.  That leaves us with gcc.target/i386/mvc2.c, which is sort of
weird because it emits no code at all.  No idea if this intended,
though.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2017-07-03  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* gcc.target/i386/mvc6.c: Require ifunc support.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: testsuite-i386-mvc6-ifunc.patch --]
[-- Type: text/x-patch, Size: 496 bytes --]

# HG changeset patch
# Parent  69f342cb37ffd9b438e9a09ca3ad5692c2aa1dec
Require ifunc support in gcc.target/i386/mvc6.c

diff --git a/gcc/testsuite/gcc.target/i386/mvc6.c b/gcc/testsuite/gcc.target/i386/mvc6.c
--- a/gcc/testsuite/gcc.target/i386/mvc6.c
+++ b/gcc/testsuite/gcc.target/i386/mvc6.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-require-ifunc "" } */
 /* { dg-options "-O3" } */
 /* { dg-final { scan-assembler "vpshufb" } } */
 /* { dg-final { scan-assembler "punpcklbw" } } */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix removal of ifunc (PR ipa/81214).
  2017-07-03 13:44 ` Rainer Orth
@ 2017-07-04  7:42   ` Martin Liška
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Liška @ 2017-07-04  7:42 UTC (permalink / raw)
  To: Rainer Orth; +Cc: gcc-patches, Michael Meissner, Dominique d'Humieres

[-- Attachment #1: Type: text/plain, Size: 2680 bytes --]

On 07/03/2017 03:44 PM, Rainer Orth wrote:
> Hi Martin,
> 
>> Following patch fixes the issue where we do not emit ifunc and resolver
>> for function that are not called in a compilation unit or and not
>> referenced.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>> i386.exp tests work on x86_64-linux-gnu.
> 
> your patch caused a testsuite regression on various targets:
> 
> FAIL: gcc.target/i386/mvc6.c (test for excess errors)
> UNRESOLVED: gcc.target/i386/mvc6.c scan-assembler punpcklbw
> UNRESOLVED: gcc.target/i386/mvc6.c scan-assembler vpshufb

Hello.

Thanks for the patch, actually I was working on the patch when you committed :)

> 
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/i386/mvc6.c:8:1: error: the call requires ifunc, which is not supported by this target
> 
> I'm seeing it on i386-pc-solaris2.12, Dominique reported it in the PR on
> Darwin/x86, and there are also testsuite results on FreeBSD/x86.
> 
> Unlike most other __attribute__((target_clones)) tests, this one lacked
> { dg-require-ifunc "" } and didn't need it before.
> 
> The following patch fixes this.  Tested with the appropriate runtest
> invocation on i386-pc-solaris2.12 and x86_64-pc-linux-gnu, installed on
> mainline.
> 
> While I was at it, I checked the other testcases with
> __attribute__((target_clones)):
> 
> g++.dg/ext/mvc1.C		dg-require-ifunc ""
> g++.dg/ext/mvc2.C		00 (dg-warning)
> g++.dg/ext/mvc3.C		00 (dg-warning)
> g++.dg/ext/mvc4.C		dg-require-ifunc ""
> gcc.dg/tree-prof/pr66295.c	dg-require-ifunc ""
> gcc.target/i386/mvc1.c		dg-require-ifunc ""
> gcc.target/i386/mvc2.c		00
> gcc.target/i386/mvc3.c		00 (dg-error)
> gcc.target/i386/mvc4.c		dg-require-ifunc ""
> gcc.target/i386/mvc5.c		dg-require-ifunc ""
> gcc.target/i386/mvc6.c		00
> gcc.target/i386/mvc7.c		dg-require-ifunc ""
> gcc.target/i386/mvc8.c		dg-require-ifunc ""
> gcc.target/i386/mvc9.c		dg-require-ifunc ""
> gcc.target/i386/pr78419.c	dg-require-ifunc ""
> gcc.target/i386/pr80732.c	dg-require-ifunc ""
> gcc.target/i386/pr81214.c	dg-require-ifunc ""
> gcc.target/powerpc/clone1.c	powerpc*-*-linux* && lp64
> 
> Of those without dg-require-ifunc, the powerpc one is (sort of) ok since
> it's restricted to Linux, and those with dg-warning/dg-error are too
> since the warnings is emitted before the error about missing ifunc
> support.  That leaves us with gcc.target/i386/mvc2.c, which is sort of
> weird because it emits no code at all.  No idea if this intended,
> though.

I prefer to add ifunc to all these tests. I'll do it in attached patch that I'm going
to install.

Thanks and sorry for the fallout.
Martin

> 
> 	Rainer
> 


[-- Attachment #2: 0001-Add-dg-require-ifunc-for-mvc-test-cases.patch --]
[-- Type: text/x-patch, Size: 2220 bytes --]

From 8b3d6abf45c6f1fdfe6780869eb0c8e6a0380f5a Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 4 Jul 2017 09:39:10 +0200
Subject: [PATCH] Add dg-require ifunc for mvc test-cases.

gcc/testsuite/ChangeLog:

2017-07-04  Martin Liska  <mliska@suse.cz>

	PR ipa/81214
	* g++.dg/ext/mvc2.C: Add dg-require ifunc.
	* g++.dg/ext/mvc3.C: Likewise.
	* gcc.target/i386/mvc2.c: Likewise.
	* gcc.target/i386/mvc3.c: Likewise.
---
 gcc/testsuite/g++.dg/ext/mvc2.C      | 1 +
 gcc/testsuite/g++.dg/ext/mvc3.C      | 1 +
 gcc/testsuite/gcc.target/i386/mvc2.c | 1 +
 gcc/testsuite/gcc.target/i386/mvc3.c | 1 +
 4 files changed, 4 insertions(+)

diff --git a/gcc/testsuite/g++.dg/ext/mvc2.C b/gcc/testsuite/g++.dg/ext/mvc2.C
index e7abab81d95..1b8c6f4d6e9 100644
--- a/gcc/testsuite/g++.dg/ext/mvc2.C
+++ b/gcc/testsuite/g++.dg/ext/mvc2.C
@@ -1,4 +1,5 @@
 /* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" } */
 
 __attribute__((target_clones("avx","arch=slm","default")))
 __attribute__((target("avx")))
diff --git a/gcc/testsuite/g++.dg/ext/mvc3.C b/gcc/testsuite/g++.dg/ext/mvc3.C
index 05bebf7d4fb..d32b2c93aa0 100644
--- a/gcc/testsuite/g++.dg/ext/mvc3.C
+++ b/gcc/testsuite/g++.dg/ext/mvc3.C
@@ -1,4 +1,5 @@
 /* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" } */
 
 __attribute__((target("avx")))
 __attribute__((target_clones("avx","arch=slm","default")))
diff --git a/gcc/testsuite/gcc.target/i386/mvc2.c b/gcc/testsuite/gcc.target/i386/mvc2.c
index 9635ec83fac..34a777c1d5e 100644
--- a/gcc/testsuite/gcc.target/i386/mvc2.c
+++ b/gcc/testsuite/gcc.target/i386/mvc2.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-require-ifunc "" } */
 
 __attribute__((target_clones("avx","arch=slm","arch=core-avx2")))
 int foo ();
diff --git a/gcc/testsuite/gcc.target/i386/mvc3.c b/gcc/testsuite/gcc.target/i386/mvc3.c
index f940cdbbf55..1c7755fabbe 100644
--- a/gcc/testsuite/gcc.target/i386/mvc3.c
+++ b/gcc/testsuite/gcc.target/i386/mvc3.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-require-ifunc "" } */
 
 __attribute__((target_clones("avx","arch=slm","arch=core-avx2")))
 int foo (); /* { dg-error "default target was not set" } */
-- 
2.13.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-04  7:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-30  8:49 [PATCH] Fix removal of ifunc (PR ipa/81214) Martin Liška
2017-06-30 12:38 ` Jan Hubicka
2017-07-03 13:44 ` Rainer Orth
2017-07-04  7:42   ` Martin Liška

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).