[PATCH, rs6000] Fix PR63354

[PATCH, rs6000] Fix PR63354

[Bill Schmidt]

Hi,

Anton Blanchard proposed a fix to his own bug report in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63354, but never submitted
the patch upstream.  I've added a formal test case and am submitting on
his behalf.

The patch simply ensures that we don't stack a frame for leaf procedures
when called with -pg -mprofile-kernel.  The automatically generated
calls to _mcount occur prior to the prolog and do not require us to
stack a frame.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  Is this ok for trunk?

Thanks,
Bill


[gcc]

2016-01-21  Anton Blanchard  <anton@samba.org>
	    Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/63354
	* config/rs6000/linux64.h (TARGET_KEEP_LEAF_WHEN_PROFILED): New
	#define.
	* config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled): New
	function.

[gcc/testsuite]

2016-01-21  Anton Blanchard  <anton@samba.org>
	    Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/63354
	* gcc.target/powerpc/pr63354.c:  New test.


Index: gcc/config/rs6000/linux64.h
===================================================================
--- gcc/config/rs6000/linux64.h	(revision 232677)
+++ gcc/config/rs6000/linux64.h	(working copy)
@@ -59,6 +59,9 @@ extern int dot_symbols;
 
 #define TARGET_PROFILE_KERNEL profile_kernel
 
+#undef TARGET_KEEP_LEAF_WHEN_PROFILED
+#define TARGET_KEEP_LEAF_WHEN_PROFILED rs6000_keep_leaf_when_profiled
+
 #define TARGET_USES_LINUX64_OPT 1
 #ifdef HAVE_LD_LARGE_TOC
 #undef TARGET_CMODEL
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 232677)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -26237,6 +26237,14 @@ rs6000_output_function_prologue (FILE *file,
   rs6000_pic_labelno++;
 }
 
+/* -mprofile-kernel code calls mcount before the function prolog,
+   so a profiled leaf function should stay a leaf function.  */
+static bool
+rs6000_keep_leaf_when_profiled ()
+{
+  return TARGET_PROFILE_KERNEL;
+}
+
 /* Non-zero if vmx regs are restored before the frame pop, zero if
    we restore after the pop when possible.  */
 #define ALWAYS_RESTORE_ALTIVEC_BEFORE_POP 0
Index: gcc/testsuite/gcc.target/powerpc/pr63354.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr63354.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr63354.c	(working copy)
@@ -0,0 +1,11 @@
+/* Verify that we don't stack a frame for leaf functions when using
+   -pg -mprofile-kernel.  */
+
+/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-options "-O2 -pg -mprofile-kernel" } */
+/* { dg-final { scan-assembler-not "mtlr" } } */
+
+int foo(void)
+{
+  return 1;
+}

Re: [PATCH, rs6000] Fix PR63354

[David Edelsohn]

On Thu, Jan 21, 2016 at 11:48 AM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> Anton Blanchard proposed a fix to his own bug report in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63354, but never submitted
> the patch upstream.  I've added a formal test case and am submitting on
> his behalf.
>
> The patch simply ensures that we don't stack a frame for leaf procedures
> when called with -pg -mprofile-kernel.  The automatically generated
> calls to _mcount occur prior to the prolog and do not require us to
> stack a frame.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2016-01-21  Anton Blanchard  <anton@samba.org>
>             Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR target/63354
>         * config/rs6000/linux64.h (TARGET_KEEP_LEAF_WHEN_PROFILED): New
>         #define.
>         * config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled): New
>         function.
>
> [gcc/testsuite]
>
> 2016-01-21  Anton Blanchard  <anton@samba.org>
>             Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR target/63354
>         * gcc.target/powerpc/pr63354.c:  New test.

Okay.

Thanks, David

Re: [PATCH, rs6000] Fix PR63354

[Bill Schmidt]

The testcase will need a slight adjustment, as currently it fails on
powerpc64 with -m32 testing.  Working on a fix.

Bill

On Thu, 2016-01-21 at 12:28 -0500, David Edelsohn wrote:
> On Thu, Jan 21, 2016 at 11:48 AM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Hi,
> >
> > Anton Blanchard proposed a fix to his own bug report in
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63354, but never submitted
> > the patch upstream.  I've added a formal test case and am submitting on
> > his behalf.
> >
> > The patch simply ensures that we don't stack a frame for leaf procedures
> > when called with -pg -mprofile-kernel.  The automatically generated
> > calls to _mcount occur prior to the prolog and do not require us to
> > stack a frame.
> >
> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> > regressions.  Is this ok for trunk?
> >
> > Thanks,
> > Bill
> >
> >
> > [gcc]
> >
> > 2016-01-21  Anton Blanchard  <anton@samba.org>
> >             Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >
> >         PR target/63354
> >         * config/rs6000/linux64.h (TARGET_KEEP_LEAF_WHEN_PROFILED): New
> >         #define.
> >         * config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled): New
> >         function.
> >
> > [gcc/testsuite]
> >
> > 2016-01-21  Anton Blanchard  <anton@samba.org>
> >             Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >
> >         PR target/63354
> >         * gcc.target/powerpc/pr63354.c:  New test.
> 
> Okay.
> 
> Thanks, David
> 

Re: [PATCH, rs6000] Fix PR63354

[Bill Schmidt]

Hi,

On Thu, 2016-01-21 at 21:21 -0600, Bill Schmidt wrote:
> The testcase will need a slight adjustment, as currently it fails on
> powerpc64 with -m32 testing.  Working on a fix.
> 
> Bill
> 

This patch adjusts the gcc.target/powerpc/pr63354 test to require 64-bit
code generation, and also restricts the test to Linux targets, as this
is necessary for using -mprofile-kernel.  Tested on
powerpc64-unknown-linux-gnu configured with --with-cpu=power7 and
testing with -m32; the test is now correctly skipped there.  Is this
okay for trunk?

Thanks,
Bill


2016-01-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* gcc.target/powerpc/pr63354.c: Restrict to Linux targets with
	64-bit support.


Index: gcc/testsuite/gcc.target/powerpc/pr63354.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr63354.c	(revision 232716)
+++ gcc/testsuite/gcc.target/powerpc/pr63354.c	(working copy)
@@ -1,8 +1,9 @@
 /* Verify that we don't stack a frame for leaf functions when using
    -pg -mprofile-kernel.  */
 
-/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-do compile { target { powerpc64*-linux-* } } } */
 /* { dg-options "-O2 -pg -mprofile-kernel" } */
+/* { dg-require-effective-target powerpc64 } */
 /* { dg-final { scan-assembler-not "mtlr" } } */
 
 int foo(void)

Re: [PATCH, rs6000] Fix PR63354

[David Edelsohn]

On Fri, Jan 22, 2016 at 12:42 AM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> On Thu, 2016-01-21 at 21:21 -0600, Bill Schmidt wrote:
>> The testcase will need a slight adjustment, as currently it fails on
>> powerpc64 with -m32 testing.  Working on a fix.
>>
>> Bill
>>
>
> This patch adjusts the gcc.target/powerpc/pr63354 test to require 64-bit
> code generation, and also restricts the test to Linux targets, as this
> is necessary for using -mprofile-kernel.  Tested on
> powerpc64-unknown-linux-gnu configured with --with-cpu=power7 and
> testing with -m32; the test is now correctly skipped there.  Is this
> okay for trunk?
>
> Thanks,
> Bill
>
>
> 2016-01-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * gcc.target/powerpc/pr63354.c: Restrict to Linux targets with
>         64-bit support.

Okay.

Thanks, David

Re: [PATCH, rs6000] Fix PR63354

[Joseph Myers]

On Thu, 21 Jan 2016, Bill Schmidt wrote:

> +/* { dg-do compile { target { powerpc64*-linux-* } } } */

That's suboptimal; you should allow powerpc*-*-linux* targets so that the 
test is also run for --enable-targets=all powerpc-linux builds when 
testing a -m64 multilib.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH, rs6000] Fix PR63354

[Bill Schmidt]

OK, thanks, Joseph!  I'll make that adjustment later today.

Bill

On Fri, 2016-01-22 at 15:51 +0000, Joseph Myers wrote:
> On Thu, 21 Jan 2016, Bill Schmidt wrote:
> 
> > +/* { dg-do compile { target { powerpc64*-linux-* } } } */
> 
> That's suboptimal; you should allow powerpc*-*-linux* targets so that the 
> test is also run for --enable-targets=all powerpc-linux builds when 
> testing a -m64 multilib.
> 

Re: [PATCH, rs6000] Fix PR63354

[Jan-Benedict Glaw]

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

On Thu, 2016-01-21 23:42:40 -0600, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
> On Thu, 2016-01-21 at 21:21 -0600, Bill Schmidt wrote:
> > The testcase will need a slight adjustment, as currently it fails on
> > powerpc64 with -m32 testing.  Working on a fix.
> 
> This patch adjusts the gcc.target/powerpc/pr63354 test to require 64-bit
> code generation, and also restricts the test to Linux targets, as this
> is necessary for using -mprofile-kernel.  Tested on
> powerpc64-unknown-linux-gnu configured with --with-cpu=power7 and
> testing with -m32; the test is now correctly skipped there.  Is this
> okay for trunk?

Building for --target=powerpc-xilinx-eabi, I see this on my build
robot (see at the bottom of the page, the make.out artifact of build
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=483851):

g++ -fno-PIE -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace   -o rs6000.o -MT rs6000.o -MMD -MP -MF ./.deps/rs6000.TPo ../../../gcc/gcc/config/rs6000/rs6000.c
../../../gcc/gcc/config/rs6000/rs6000.c:26243:1: error: ‘bool rs6000_keep_leaf_when_profiled()’ defined but not used [-Werror=unused-function]
 rs6000_keep_leaf_when_profiled ()
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

cc1plus: all warnings being treated as errors
Makefile:2121: recipe for target 'rs6000.o' failed
make[2]: *** [rs6000.o] Error 1
make[2]: Leaving directory '/home/jbglaw/build-configlist_mk/powerpc-xilinx-eabi/build-gcc/mk/powerpc-xilinx-eabi/gcc'
Makefile:4123: recipe for target 'all-gcc' failed
make[1]: *** [all-gcc] Error 2
make[1]: Leaving directory '/home/jbglaw/build-configlist_mk/powerpc-xilinx-eabi/build-gcc/mk/powerpc-xilinx-eabi'


MfG, JBG

-- 
      Jan-Benedict Glaw      jbglaw@lug-owl.de              +49-172-7608481
Signature of:              Träume nicht von Deinem Leben: Lebe Deinen Traum!
the second  :

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH, rs6000] Fix PR63354

[Bill Schmidt]

On Sun, 2016-01-24 at 02:18 +0100, Jan-Benedict Glaw wrote:
> On Thu, 2016-01-21 23:42:40 -0600, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
> > On Thu, 2016-01-21 at 21:21 -0600, Bill Schmidt wrote:
> > > The testcase will need a slight adjustment, as currently it fails on
> > > powerpc64 with -m32 testing.  Working on a fix.
> > 
> > This patch adjusts the gcc.target/powerpc/pr63354 test to require 64-bit
> > code generation, and also restricts the test to Linux targets, as this
> > is necessary for using -mprofile-kernel.  Tested on
> > powerpc64-unknown-linux-gnu configured with --with-cpu=power7 and
> > testing with -m32; the test is now correctly skipped there.  Is this
> > okay for trunk?
> 
> Building for --target=powerpc-xilinx-eabi, I see this on my build
> robot (see at the bottom of the page, the make.out artifact of build
> http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=483851):
> 
> g++ -fno-PIE -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace   -o rs6000.o -MT rs6000.o -MMD -MP -MF ./.deps/rs6000.TPo ../../../gcc/gcc/config/rs6000/rs6000.c
> ../../../gcc/gcc/config/rs6000/rs6000.c:26243:1: error: ‘bool rs6000_keep_leaf_when_profiled()’ defined but not used [-Werror=unused-function]
>  rs6000_keep_leaf_when_profiled ()
>  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> cc1plus: all warnings being treated as errors
> Makefile:2121: recipe for target 'rs6000.o' failed
> make[2]: *** [rs6000.o] Error 1
> make[2]: Leaving directory '/home/jbglaw/build-configlist_mk/powerpc-xilinx-eabi/build-gcc/mk/powerpc-xilinx-eabi/gcc'
> Makefile:4123: recipe for target 'all-gcc' failed
> make[1]: *** [all-gcc] Error 2
> make[1]: Leaving directory '/home/jbglaw/build-configlist_mk/powerpc-xilinx-eabi/build-gcc/mk/powerpc-xilinx-eabi'
> 
> 
> MfG, JBG
> 

Hi Jan, thanks for the report!  Patch below that should fix the problem.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu, no
regressions.  David, is this ok for trunk?

Thanks,
Bill


2016-01-24  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled):  Add
	decl with __attribute__ ((unused)) annotation.


Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 232783)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -1311,6 +1311,7 @@ static bool rs6000_secondary_reload_move (enum rs6
 					  secondary_reload_info *,
 					  bool);
 rtl_opt_pass *make_pass_analyze_swaps (gcc::context*);
+static bool rs6000_keep_leaf_when_profiled () __attribute__ ((unused));
 
 /* Hash table stuff for keeping track of TOC entries.  */
 

Re: [PATCH, rs6000] Fix PR63354

[David Edelsohn]

On Sun, Jan 24, 2016 at 9:17 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:

> Hi Jan, thanks for the report!  Patch below that should fix the problem.
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu, no
> regressions.  David, is this ok for trunk?
>
> Thanks,
> Bill
>
>
> 2016-01-24  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled):  Add
>         decl with __attribute__ ((unused)) annotation.

Okay.

Thanks, David

Re: [PATCH, rs6000] Fix PR63354

[Bill Schmidt]

Thanks, committed as r232793.

Bill

On Mon, 2016-01-25 at 08:54 -0500, David Edelsohn wrote:
> On Sun, Jan 24, 2016 at 9:17 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> 
> > Hi Jan, thanks for the report!  Patch below that should fix the problem.
> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu, no
> > regressions.  David, is this ok for trunk?
> >
> > Thanks,
> > Bill
> >
> >
> > 2016-01-24  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >
> >         * config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled):  Add
> >         decl with __attribute__ ((unused)) annotation.
> 
> Okay.
> 
> Thanks, David
> 

Re: [PATCH, rs6000] Fix PR63354

[Andreas Schwab]

Bill Schmidt <wschmidt@linux.vnet.ibm.com> writes:

> Index: gcc/testsuite/gcc.target/powerpc/pr63354.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/pr63354.c	(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/pr63354.c	(working copy)
> @@ -0,0 +1,11 @@
> +/* Verify that we don't stack a frame for leaf functions when using
> +   -pg -mprofile-kernel.  */
> +
> +/* { dg-do compile { target { powerpc64*-*-* } } } */
> +/* { dg-options "-O2 -pg -mprofile-kernel" } */
> +/* { dg-final { scan-assembler-not "mtlr" } } */
> +
> +int foo(void)
> +{
> +  return 1;
> +}

With -m32:

FAIL: gcc.target/powerpc/pr63354.c (test for excess errors)
Excess errors:
/daten/gcc/gcc-20160205/gcc/testsuite/gcc.target/powerpc/pr63354.c:1:0: error: -mprofile-kernel not supported in this configuration

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH, rs6000] Fix PR63354

[Bill Schmidt]

Hi Andreas,

Sorry I haven't responded sooner; I was on vacation and have been
unpiling things since then.  The test case had already been updated
since the patch you cited, adding

/* { dg-require-effective-target powerpc64 } */

Is this the version you're testing with?

Thanks,
Bill

On Sat, 2016-02-06 at 21:35 +0100, Andreas Schwab wrote:
> Bill Schmidt <wschmidt@linux.vnet.ibm.com> writes:
> 
> > Index: gcc/testsuite/gcc.target/powerpc/pr63354.c
> > ===================================================================
> > --- gcc/testsuite/gcc.target/powerpc/pr63354.c	(revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/pr63354.c	(working copy)
> > @@ -0,0 +1,11 @@
> > +/* Verify that we don't stack a frame for leaf functions when using
> > +   -pg -mprofile-kernel.  */
> > +
> > +/* { dg-do compile { target { powerpc64*-*-* } } } */
> > +/* { dg-options "-O2 -pg -mprofile-kernel" } */
> > +/* { dg-final { scan-assembler-not "mtlr" } } */
> > +
> > +int foo(void)
> > +{
> > +  return 1;
> > +}
> 
> With -m32:
> 
> FAIL: gcc.target/powerpc/pr63354.c (test for excess errors)
> Excess errors:
> /daten/gcc/gcc-20160205/gcc/testsuite/gcc.target/powerpc/pr63354.c:1:0: error: -mprofile-kernel not supported in this configuration
> 
> Andreas.
>