[PATCHv2][PR target/81535] Fix tests on Power

[PATCHv2][PR target/81535] Fix tests on Power

[Yury Gribov]

[-- 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

Re: [PATCHv2][PR target/81535] Fix tests on Power

[Yury Gribov]

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

Re: [PATCHv2][PR target/81535] Fix tests on Power

[Segher Boessenkool]

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