* [PATCHv2][PR target/81535] Fix tests on Power
@ 2018-02-15 20:13 Yury Gribov
2018-02-16 8:56 ` Yury Gribov
2018-02-16 15:27 ` Segher Boessenkool
0 siblings, 2 replies; 3+ messages in thread
From: Yury Gribov @ 2018-02-15 20:13 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, seurer, wschmidt, meissner, marxin
[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]
Hi all,
This is an updated version of patch for PR 81535. The patch was last
discussed in November. The new version addresses Segher's comments in
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00558.html
The patch was tested on
x86_64-pc-linux-gnu
powerpc64-unknown-linux-gnu
powerpc64le-unknown-linux-gnu
by running
$ make check-gcc RUNTESTFLAGS='powerpc.exp=pr79439-*.c dg.exp=pr56727-*.c '
$ make check-c RUNTESTFLAGS='powerpc.exp=pr79439-*.c
dg.exp=pr56727-*.c --target_board="unix/-m32"'
(I didn't perform full testing as I don't change anything in compiler).
FTR here's an excerpt from
https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02231.html which
explains the reasoning behind PR 56727 (which introduced regression in
PR 81535):
>> Is it correct that current GCC does not do the call via the PLT?
>
> Well, it was decided in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56727 that it would be a
> valid optimization because the only way to expose the difference would
> be through dlsym hackery. Note that original PowerPC use-case
> (reported in https://sourceware.org/bugzilla/show_bug.cgi?id=21116)
> would benefit from this optimization as because PLT call + indirection
> would be replaced by normal PC-relative call.
-Y
[-- Attachment #2: pr81535-2.patch --]
[-- Type: application/octet-stream, Size: 6950 bytes --]
From 68ef3663aed0bb6f33c0c14e98544f7f7b797469 Mon Sep 17 00:00:00 2001
From: Yury Gribov <tetra2005@gmail.com>
Date: Thu, 15 Feb 2018 07:48:57 +0000
Subject: [PATCH] Updated PowerPC tests.
gcc/testsuite/
2018-02-15 Yury Gribov <tetra2005@gmail.com>
PR target/81535
* gcc.dg/pr56727-1.c: Prevent tailcalls and update for powerpc*-*-*.
* gcc.dg/pr56727-2.c: Ditto.
* gcc.target/powerpc/pr79439.c: Renamed to...
* gcc.target/powerpc/pr79439-1.c: ...this.
* gcc.target/powerpc/pr79439-2.c: New test.
* gcc.target/powerpc/pr79439-3.c: New test.
---
gcc/testsuite/gcc.dg/pr56727-1.c | 5 ++++-
gcc/testsuite/gcc.dg/pr56727-2.c | 6 +++++-
gcc/testsuite/gcc.target/powerpc/pr79439-1.c | 28 +++++++++++++++++++++++++
gcc/testsuite/gcc.target/powerpc/pr79439-2.c | 31 ++++++++++++++++++++++++++++
gcc/testsuite/gcc.target/powerpc/pr79439-3.c | 25 ++++++++++++++++++++++
gcc/testsuite/gcc.target/powerpc/pr79439.c | 29 --------------------------
6 files changed, 93 insertions(+), 31 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr79439-1.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr79439-2.c
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr79439-3.c
delete mode 100644 gcc/testsuite/gcc.target/powerpc/pr79439.c
diff --git a/gcc/testsuite/gcc.dg/pr56727-1.c b/gcc/testsuite/gcc.dg/pr56727-1.c
index ffc1335..a26edb2 100644
--- a/gcc/testsuite/gcc.dg/pr56727-1.c
+++ b/gcc/testsuite/gcc.dg/pr56727-1.c
@@ -1,6 +1,5 @@
/* { dg-do compile { target fpic } } */
/* { dg-options "-O2 -fPIC" } */
-/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-* x86_64-*-* powerpc*-*-* } } } */
#define define_func(type) \
void f_ ## type (type b) { f_ ## type (0); } \
@@ -21,3 +20,7 @@ int __attribute__((noinline, noclone)) foo_noinline(int n)
{
return (n == 1 || n == 2) ? 1 : foo_noinline(n-1) * foo_noinline(n-2);
}
+
+/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-* x86_64-*-* } } } */
+/* { dg-final { scan-assembler-not "@(PLT|plt)" { target { powerpc*-*-* && ilp32 } } } } */
+/* { dg-final { scan-assembler-not "bl \[a-z_\]*\n\\s*nop" { target { powerpc*-*-* && lp64 } } } } */
diff --git a/gcc/testsuite/gcc.dg/pr56727-2.c b/gcc/testsuite/gcc.dg/pr56727-2.c
index 62a74d1..c54369e 100644
--- a/gcc/testsuite/gcc.dg/pr56727-2.c
+++ b/gcc/testsuite/gcc.dg/pr56727-2.c
@@ -1,11 +1,11 @@
/* { dg-do compile { target fpic } } */
/* { dg-options "-O2 -fPIC" } */
/* { dg-require-alias "" } */
-/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* powerpc*-*-linux* } } } */
__attribute__((noinline, noclone))
void f (short b)
{
+ __builtin_setjmp (0); /* Prevent tailcall */
f (0);
}
@@ -15,3 +15,7 @@ void h ()
{
g (0);
}
+
+/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* } } } */
+/* { dg-final { scan-assembler "@(PLT|plt)" { target { powerpc*-*-linux* && ilp32 } } } } */
+/* { dg-final { scan-assembler "bl f\n\\s*nop" { target { powerpc*-*-linux* && lp64 } } } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79439-1.c b/gcc/testsuite/gcc.target/powerpc/pr79439-1.c
new file mode 100644
index 0000000..14f57a2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr79439-1.c
@@ -0,0 +1,28 @@
+/* { dg-do compile { target { powerpc64*-*-linux* && lp64 } } } */
+/* { dg-options "-O2 -fpic -fno-reorder-blocks" } */
+
+/* On the Linux 64-bit ABIs, we eliminate NOP in the 'rec' call even if
+ -fpic is used. The recursive call should call the local alias. The
+ Linux 32-bit ABIs do not require NOPs after the BL instruction. */
+
+int f (void);
+
+void
+g (void)
+{
+}
+
+int
+rec (int a)
+{
+ int ret = 0;
+ if (a > 10 && f ())
+ ret += rec (a - 1);
+ g ();
+ return a + ret;
+}
+
+/* { dg-final { scan-assembler-times {\mbl f\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mbl g\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mnop\M} 2 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79439-2.c b/gcc/testsuite/gcc.target/powerpc/pr79439-2.c
new file mode 100644
index 0000000..26e0e0e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr79439-2.c
@@ -0,0 +1,31 @@
+/* { dg-do compile { target { powerpc64*-*-linux* && lp64 } } } */
+/* { dg-options "-O2 -fpic -fno-reorder-blocks" } */
+
+/* On the Linux 64-bit ABIs, we should not eliminate NOP in the 'rec' call if
+ -fpic is used because rec can be interposed at link time (since it has an
+ alias), and the recursive call should call the interposed function. The
+ Linux 32-bit ABIs do not require NOPs after the BL instruction. */
+
+int f (void);
+
+void
+g (void)
+{
+}
+
+int
+rec (int a)
+{
+ int ret = 0;
+ if (a > 10 && f ())
+ ret += rec (a - 1);
+ g ();
+ return a + ret;
+}
+
+int rec_alias (int) __attribute__ ((alias ("rec")));
+
+/* { dg-final { scan-assembler-times {\mbl f\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mbl g\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mnop\M} 3 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79439-3.c b/gcc/testsuite/gcc.target/powerpc/pr79439-3.c
new file mode 100644
index 0000000..07162ab
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr79439-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile { target { powerpc64-*-linux* && ilp32 } } } */
+/* { dg-options "-O2 -fpic -fno-reorder-blocks" } */
+
+/* Analog of pr79439-1.c for 32-bit Linux. */
+
+int f (void);
+
+void
+g (void)
+{
+}
+
+int
+rec (int a)
+{
+ int ret = 0;
+ if (a > 10 && f ())
+ ret += rec (a - 1);
+ g ();
+ return a + ret;
+}
+
+/* { dg-final { scan-assembler-times {\mbl f@plt\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mbl g@plt\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mbl rec@plt\M} 0 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/pr79439.c b/gcc/testsuite/gcc.target/powerpc/pr79439.c
deleted file mode 100644
index 23c9a24..0000000
--- a/gcc/testsuite/gcc.target/powerpc/pr79439.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/* { dg-do compile { target { powerpc64*-*-linux* && lp64 } } } */
-/* { dg-options "-O2 -fpic" } */
-
-/* On the Linux 64-bit ABIs, we should not eliminate NOP in the 'rec' call if
- -fpic is used because rec can be interposed at link time (since it is
- external), and the recursive call should call the interposed function. The
- Linux 32-bit ABIs do not require NOPs after the BL instruction. */
-
-int f (void);
-
-void
-g (void)
-{
-}
-
-int
-rec (int a)
-{
- int ret = 0;
- if (a > 10 && f ())
- ret += rec (a - 1);
- g ();
- return a + ret;
-}
-
-/* { dg-final { scan-assembler-times {\mbl f\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mbl g\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
-/* { dg-final { scan-assembler-times {\mnop\M} 3 } } */
--
1.8.3.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2][PR target/81535] Fix tests on Power
2018-02-15 20:13 [PATCHv2][PR target/81535] Fix tests on Power Yury Gribov
@ 2018-02-16 8:56 ` Yury Gribov
2018-02-16 15:27 ` Segher Boessenkool
1 sibling, 0 replies; 3+ messages in thread
From: Yury Gribov @ 2018-02-16 8:56 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, seurer, wschmidt, meissner, marxin
On Thu, Feb 15, 2018 at 8:13 PM, Yury Gribov
<tetra2005.patches@gmail.com> wrote:
> Hi all,
>
> This is an updated version of patch for PR 81535. The patch was last
> discussed in November. The new version addresses Segher's comments in
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00558.html
Forgot to mention: I've added the -fno-reorder-blocks flag to PowerPC
tests because otherwise bbro pass copies call to g() which breaks
number of nops. Roughly
if (a > 10 && f ())
ret += rec (a - 1);
g ();
is converted to
if (a <= 10) {
lab:
g();
} else {
if (!f())
goto lab;
ret += rec (a - 1);
g ();
}
-Y
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2][PR target/81535] Fix tests on Power
2018-02-15 20:13 [PATCHv2][PR target/81535] Fix tests on Power Yury Gribov
2018-02-16 8:56 ` Yury Gribov
@ 2018-02-16 15:27 ` Segher Boessenkool
1 sibling, 0 replies; 3+ messages in thread
From: Segher Boessenkool @ 2018-02-16 15:27 UTC (permalink / raw)
To: Yury Gribov; +Cc: GCC Patches, seurer, wschmidt, meissner, marxin
Hi Yuri,
On Thu, Feb 15, 2018 at 08:13:38PM +0000, Yury Gribov wrote:
> This is an updated version of patch for PR 81535. The patch was last
> discussed in November. The new version addresses Segher's comments in
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00558.html
> FTR here's an excerpt from
> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02231.html which
> explains the reasoning behind PR 56727 (which introduced regression in
> PR 81535):
>
> >> Is it correct that current GCC does not do the call via the PLT?
> >
> > Well, it was decided in
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56727 that it would be a
> > valid optimization because the only way to expose the difference would
> > be through dlsym hackery. Note that original PowerPC use-case
> > (reported in https://sourceware.org/bugzilla/show_bug.cgi?id=21116)
> > would benefit from this optimization as because PLT call + indirection
> > would be replaced by normal PC-relative call.
Yup.
> From 68ef3663aed0bb6f33c0c14e98544f7f7b797469 Mon Sep 17 00:00:00 2001
> From: Yury Gribov <tetra2005@gmail.com>
> Date: Thu, 15 Feb 2018 07:48:57 +0000
> Subject: [PATCH] Updated PowerPC tests.
>
> gcc/testsuite/
>
> 2018-02-15 Yury Gribov <tetra2005@gmail.com>
>
> PR target/81535
> * gcc.dg/pr56727-1.c: Prevent tailcalls and update for powerpc*-*-*.
> * gcc.dg/pr56727-2.c: Ditto.
> * gcc.target/powerpc/pr79439.c: Renamed to...
> * gcc.target/powerpc/pr79439-1.c: ...this.
> * gcc.target/powerpc/pr79439-2.c: New test.
> * gcc.target/powerpc/pr79439-3.c: New test.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr79439-1.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile { target { powerpc64*-*-linux* && lp64 } } } */
> +/* { dg-options "-O2 -fpic -fno-reorder-blocks" } */
Please do
/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
that is, compile on _all_ powerpc, whether the default target of the
compiler is 64-bit or not (we only care about the *current* target).
Same for the other tests.
Okay for trunk with that change. Thanks for working on this!
Segher
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-16 15:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-15 20:13 [PATCHv2][PR target/81535] Fix tests on Power Yury Gribov
2018-02-16 8:56 ` Yury Gribov
2018-02-16 15:27 ` Segher Boessenkool
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).