public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
@ 2015-03-13 21:55 Martin Sebor
  2015-03-14 14:34 ` Segher Boessenkool
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Sebor @ 2015-03-13 21:55 UTC (permalink / raw)
  To: Gcc Patch List; +Cc: anton

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

Attached is a patch that eliminates the unused stack frame
allocated by gcc 5 with -pg -mprofile-kernel on powepc64le
and brings the code into parity with previous gcc versions.

The patch doesn't do anything to change the emitted code
when -mprofile-kernel is used without -pg. Since the former
option isn't fully documented (as noted in pr 65372) it's
unclear what effect it should be expected to have without
-pg.

The patch was tested on {powerpc64,powerpc64le}-linux.

Martin

[-- Attachment #2: gcc-63354.patch --]
[-- Type: text/x-patch, Size: 1914 bytes --]

2015-03-13  Anton Blanchard  <anton@samba.org>

	PR target/63354
	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
	* cc/config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled). New
	function.

2015-03-13  Martin Sebor  <msebor@redhat.com>

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

diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
index 0879e7e..f51e892 100644
--- a/gcc/config/rs6000/linux64.h
+++ b/gcc/config/rs6000/linux64.h
@@ -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
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 3171eef..77731c56 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24395,6 +24395,15 @@ 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 (void)
+{
+  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
diff --git a/gcc/testsuite/gcc.target/powerpc/pr63354.c b/gcc/testsuite/gcc.target/powerpc/pr63354.c
new file mode 100644
index 0000000..dd6ad08
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-options "-O2 -pg -mprofile-kernel" } */
+
+int foo (void)
+{
+  return 1;
+}
+
+/* { dg-final { scan-assembler "bl _mcount" } } */
+/* { dg-final { scan-assembler-not "\(addi|stdu\) 1," } } */

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-13 21:55 [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le Martin Sebor
@ 2015-03-14 14:34 ` Segher Boessenkool
  2015-03-15 23:40   ` Martin Sebor
  0 siblings, 1 reply; 9+ messages in thread
From: Segher Boessenkool @ 2015-03-14 14:34 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Gcc Patch List, anton

On Fri, Mar 13, 2015 at 03:54:57PM -0600, Martin Sebor wrote:
> Attached is a patch that eliminates the unused stack frame
> allocated by gcc 5 with -pg -mprofile-kernel on powepc64le
> and brings the code into parity with previous gcc versions.
> 
> The patch doesn't do anything to change the emitted code
> when -mprofile-kernel is used without -pg. Since the former
> option isn't fully documented (as noted in pr 65372) it's
> unclear what effect it should be expected to have without
> -pg.

-mprofile-kernel does nothing without profiling enabled.  Maybe it
should just have been called -pk or something horrid like that.

The effect it should have is to do what the only user of the option
(the 64-bit PowerPC Linux kernel) wants.  The effect it does have
is to make the 64-bit ABI more like the 32-bit ABI for mcount.


> 2015-03-13  Anton Blanchard  <anton@samba.org>
> 
> 	PR target/63354
> 	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
                                       ^ typo

> 	* cc/config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled). New
          ^ typo                                                    ^ typo

It shouldn't have "gcc/" in the path names at all, actually.

> +/* -mprofile-kernel code calls mcount before the function prolog,

"prologue".

> +   so a profiled leaf function should stay a leaf function.  */
> +
> +static bool
> +rs6000_keep_leaf_when_profiled (void)
> +{
> +  return TARGET_PROFILE_KERNEL;
> +}

Something like

  switch (DEFAULT_ABI)
    {
      case ABI_AIX:
      case ABI_ELFv2:
	return TARGET_PROFILE_KERNEL;

      default:
	return true;
    }

although I'm not sure about Darwin here.  More conservative is to
return false for anything untested, of course.


> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
> +/* { dg-options "-O2 -pg -mprofile-kernel" } */
> +
> +int foo (void)
> +{
> +  return 1;
> +}
> +
> +/* { dg-final { scan-assembler "bl _mcount" } } */
> +/* { dg-final { scan-assembler-not "\(addi|stdu\) 1," } } */

Either you should run this only on AIX/ELFv2 ABIs, or you want to
test for "stwu" as well.  Bare "1" does not work for all assemblers
(only Darwin again?)


Segher

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-14 14:34 ` Segher Boessenkool
@ 2015-03-15 23:40   ` Martin Sebor
  2015-03-17 22:34     ` Segher Boessenkool
  2015-03-21 19:48     ` Iain Sandoe
  0 siblings, 2 replies; 9+ messages in thread
From: Martin Sebor @ 2015-03-15 23:40 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Gcc Patch List, anton

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

On 03/14/2015 08:34 AM, Segher Boessenkool wrote:
> On Fri, Mar 13, 2015 at 03:54:57PM -0600, Martin Sebor wrote:
>> Attached is a patch that eliminates the unused stack frame
>> allocated by gcc 5 with -pg -mprofile-kernel on powepc64le
>> and brings the code into parity with previous gcc versions.
>>
>> The patch doesn't do anything to change the emitted code
>> when -mprofile-kernel is used without -pg. Since the former
>> option isn't fully documented (as noted in pr 65372) it's
>> unclear what effect it should be expected to have without
>> -pg.
>
> -mprofile-kernel does nothing without profiling enabled.  Maybe it
> should just have been called -pk or something horrid like that.
>
> The effect it should have is to do what the only user of the option
> (the 64-bit PowerPC Linux kernel) wants.  The effect it does have
> is to make the 64-bit ABI more like the 32-bit ABI for mcount.

Thanks for the review and the clarification. FWIW, I mentioned
-pg because the reporter had noted that in prior versions of
GCC specifying -pg in addition to -mprofile-kernel wasn't
necessary to get the expected effect.

>
>
>> 2015-03-13  Anton Blanchard  <anton@samba.org>
>>
>> 	PR target/63354
>> 	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
>                                         ^ typo
>
>> 	* cc/config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled). New
>            ^ typo                                                    ^ typo
>
> It shouldn't have "gcc/" in the path names at all, actually.

Sorry, I must have mangled the ChangeLog sopmehow while copying
it from one terminal to another. I fixed it in the new patch
(attached) along with the other issues you pointed out.

I tested the changes in powerpc64*-linux-* native builds and on
an x86_64 host in a build for the powerpc-unknown-linux-gnu and
powerpc64-apple-darwin targets. Of these, the -mprofile-kernel
option is only accepted for powerpc64*-linux-* (which was also
confirmed by inspecting the sources) so I adjusted the test
target accordingly and kept the body of
rs6000_keep_leaf_when_profiled you suggested.

Martin

>
>> +/* -mprofile-kernel code calls mcount before the function prolog,
>
> "prologue".
>
>> +   so a profiled leaf function should stay a leaf function.  */
>> +
>> +static bool
>> +rs6000_keep_leaf_when_profiled (void)
>> +{
>> +  return TARGET_PROFILE_KERNEL;
>> +}
>
> Something like
>
>    switch (DEFAULT_ABI)
>      {
>        case ABI_AIX:
>        case ABI_ELFv2:
> 	return TARGET_PROFILE_KERNEL;
>
>        default:
> 	return true;
>      }
>
> although I'm not sure about Darwin here.  More conservative is to
> return false for anything untested, of course.
>
>
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile { target { powerpc*-*-* } } } */
>> +/* { dg-options "-O2 -pg -mprofile-kernel" } */
>> +
>> +int foo (void)
>> +{
>> +  return 1;
>> +}
>> +
>> +/* { dg-final { scan-assembler "bl _mcount" } } */
>> +/* { dg-final { scan-assembler-not "\(addi|stdu\) 1," } } */
>
> Either you should run this only on AIX/ELFv2 ABIs, or you want to
> test for "stwu" as well.  Bare "1" does not work for all assemblers
> (only Darwin again?)
>
>
> Segher
>


[-- Attachment #2: gcc-63354.patch --]
[-- Type: text/x-patch, Size: 2040 bytes --]

2015-03-13  Anton Blanchard  <anton@samba.org>

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

2015-03-13  Martin Sebor  <msebor@redhat.com>

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

diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
index 0879e7e..f51e892 100644
--- a/gcc/config/rs6000/linux64.h
+++ b/gcc/config/rs6000/linux64.h
@@ -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
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 31b46ea..9bad535 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24397,6 +24397,23 @@ rs6000_output_function_prologue (FILE *file,
   rs6000_pic_labelno++;
 }
 
+/* -mprofile-kernel code calls mcount before the function prologue,
+   so a profiled leaf function should stay a leaf function.  */
+
+static bool
+rs6000_keep_leaf_when_profiled (void)
+{
+  switch (DEFAULT_ABI)
+    {
+      case ABI_AIX:
+      case ABI_ELFv2:
+       return TARGET_PROFILE_KERNEL;
+
+      default:
+       return true;
+    }
+}
+
 /* 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
diff --git a/gcc/testsuite/gcc.target/powerpc/pr63354.c b/gcc/testsuite/gcc.target/powerpc/pr63354.c
new file mode 100644
index 0000000..d95f1eb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc64*-linux* } } } */
+/* { dg-options "-O2 -pg -mprofile-kernel" } */
+
+int foo (void)
+{
+  return 1;
+}
+
+/* { dg-final { scan-assembler "bl _mcount" } } */
+/* { dg-final { scan-assembler-not "\(addi|stdu\) 1," } } */

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-15 23:40   ` Martin Sebor
@ 2015-03-17 22:34     ` Segher Boessenkool
  2015-03-21 19:48     ` Iain Sandoe
  1 sibling, 0 replies; 9+ messages in thread
From: Segher Boessenkool @ 2015-03-17 22:34 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Gcc Patch List, anton

Hi Martin,

Sorry for the late answer...

On Sun, Mar 15, 2015 at 05:39:59PM -0600, Martin Sebor wrote:
> I tested the changes in powerpc64*-linux-* native builds and on
> an x86_64 host in a build for the powerpc-unknown-linux-gnu and
> powerpc64-apple-darwin targets. Of these, the -mprofile-kernel
> option is only accepted for powerpc64*-linux-* (which was also
> confirmed by inspecting the sources) so I adjusted the test
> target accordingly and kept the body of
> rs6000_keep_leaf_when_profiled you suggested.

That looks fine to me for stage 1.  The comment could use some
updating though ;-)


Segher

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-15 23:40   ` Martin Sebor
  2015-03-17 22:34     ` Segher Boessenkool
@ 2015-03-21 19:48     ` Iain Sandoe
  2015-03-24  2:50       ` Martin Sebor
  1 sibling, 1 reply; 9+ messages in thread
From: Iain Sandoe @ 2015-03-21 19:48 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Segher Boessenkool, Gcc Patch List, anton

Hi Martin,

I've applied your latest patch to top of trunk and looked at the code gen on powerpc-darwin9 (and a cross from x86-64-darwin12 => powerpc64-linux-gnu).

On 15 Mar 2015, at 23:39, Martin Sebor wrote:

> On 03/14/2015 08:34 AM, Segher Boessenkool wrote:
>> On Fri, Mar 13, 2015 at 03:54:57PM -0600, Martin Sebor wrote:
>>> Attached is a patch that eliminates the unused stack frame
>>> allocated by gcc 5 with -pg -mprofile-kernel on powepc64le
>>> and brings the code into parity with previous gcc versions.
>>> 
>>> The patch doesn't do anything to change the emitted code
>>> when -mprofile-kernel is used without -pg. Since the former
>>> option isn't fully documented (as noted in pr 65372) it's
>>> unclear what effect it should be expected to have without
>>> -pg.
>> 
>> -mprofile-kernel does nothing without profiling enabled.  Maybe it
>> should just have been called -pk or something horrid like that.
>> 
>> The effect it should have is to do what the only user of the option
>> (the 64-bit PowerPC Linux kernel) wants.  The effect it does have
>> is to make the 64-bit ABI more like the 32-bit ABI for mcount.
> 
> Thanks for the review and the clarification. FWIW, I mentioned
> -pg because the reporter had noted that in prior versions of
> GCC specifying -pg in addition to -mprofile-kernel wasn't
> necessary to get the expected effect.
> 
>> 
>> 
>>> 2015-03-13  Anton Blanchard  <anton@samba.org>
>>> 
>>> 	PR target/63354
>>> 	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
>>                                        ^ typo

This ^ will cause a bootstrap fail for every rs6000 target that doesn't include linux64.h.
(because rs6000_keep_leaf_when_profiled will be "defined but unused").

Since ISTM you intend this to apply to all rs6000 sub-targets, you might as well move it to rs6000.h?

>> 
>>> 	* cc/config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled). New
>>           ^ typo                                                    ^ typo
>> 
>> It shouldn't have "gcc/" in the path names at all, actually.
> 
> Sorry, I must have mangled the ChangeLog sopmehow while copying
> it from one terminal to another. I fixed it in the new patch
> (attached) along with the other issues you pointed out.
> 
> I tested the changes in powerpc64*-linux-* native builds and on
> an x86_64 host in a build for the powerpc-unknown-linux-gnu and
> powerpc64-apple-darwin targets. Of these, the -mprofile-kernel
> option is only accepted for powerpc64*-linux-* (which was also
> confirmed by inspecting the sources) so I adjusted the test
> target accordingly and kept the body of
> rs6000_keep_leaf_when_profiled you suggested.
> 
> Martin
> 
>> 
>>> +/* -mprofile-kernel code calls mcount before the function prolog,
>> 
>> "prologue".
>> 
>>> +   so a profiled leaf function should stay a leaf function.  */
>>> +
>>> +static bool
>>> +rs6000_keep_leaf_when_profiled (void)
>>> +{
>>> +  return TARGET_PROFILE_KERNEL;
>>> +}
>> 
>> Something like
>> 
>>   switch (DEFAULT_ABI)
>>     {
>>       case ABI_AIX:
>>       case ABI_ELFv2:
>> 	return TARGET_PROFILE_KERNEL;
>> 
>>       default:
>> 	return true;
>>     }
>> 
>> although I'm not sure about Darwin here.  More conservative is to
>> return false for anything untested, of course.

The change is 'no-op' on Darwin, since we pass a parameter to mcount a stack frame is always forced.

>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile { target { powerpc*-*-* } } } */
>>> +/* { dg-options "-O2 -pg -mprofile-kernel" } */
>>> +
>>> +int foo (void)
>>> +{
>>> +  return 1;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "bl _mcount" } } */
>>> +/* { dg-final { scan-assembler-not "\(addi|stdu\) 1," } } */
>> 
>> Either you should run this only on AIX/ELFv2 ABIs, or you want to
>> test for "stwu" as well.  Bare "1" does not work for all assemblers
>> (only Darwin again?)

a bare register # will, indeed, fail for Darwin's native assembler (which expects r#).
cheers
Iain

>> 
>> 
>> Segher
>> 
> 
> <gcc-63354.patch>

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-21 19:48     ` Iain Sandoe
@ 2015-03-24  2:50       ` Martin Sebor
  2015-03-24  8:26         ` Iain Sandoe
  2015-03-24 11:51         ` Segher Boessenkool
  0 siblings, 2 replies; 9+ messages in thread
From: Martin Sebor @ 2015-03-24  2:50 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Segher Boessenkool, Gcc Patch List, anton

On 03/21/2015 01:48 PM, Iain Sandoe wrote:
> Hi Martin,
>
> I've applied your latest patch to top of trunk and looked at the code gen on powerpc-darwin9 (and a cross from x86-64-darwin12 => powerpc64-linux-gnu).

Thanks for the review!

>>>> 2015-03-13  Anton Blanchard  <anton@samba.org>
>>>>
>>>> 	PR target/63354
>>>> 	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
>>>                                         ^ typo

It's fixed in version 2 of the patch posted here:
https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00793.html

>
> This ^ will cause a bootstrap fail for every rs6000 target that doesn't include linux64.h.
> (because rs6000_keep_leaf_when_profiled will be "defined but unused").
>
> Since ISTM you intend this to apply to all rs6000 sub-targets, you might as well move it to rs6000.h?

The powerpc-darwin9 and powerpc64-darwin9 targets both built
successfully with this patch. I also tried powerpc64-freebsd,
which succeeded as well (though I had to work around pr65535).
What target do you suggest I try to reproduce the failure?
(I don't mind moving the macro definition as you suggest,
but I'd like to understand how to trigger the problem so
that I can reproduce it and verify that I've fixed it.)

>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile { target { powerpc*-*-* } } } */
...
> a bare register # will, indeed, fail for Darwin's native assembler (which expects r#).

Thanks. I don't have access to Darwin but I changed the target
in version 2 of the patch to powerpc64*-linux* to make the test
unsupported only on Linux but now that I've tried running it on
a non-linux target I see it fails with:

dg-process-target-1: `{target powerpc64*-linux*}'
ERROR: gcc.target/powerpc/pr63354.c: syntax error in target selector 
"target powerpc64*-linux*" for " dg-do 1 compile { target { 
powerpc64*-linux* } } "

Changing the target to powerpc64*-*-linux* eliminates the
error and results in an unsupported test as intended. I'll
post a new patch with the fix for this and with one linux64.h
problem you mentioned above once I reproduce it and verify
that the suggested solution fixes it.

Martin

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-24  2:50       ` Martin Sebor
@ 2015-03-24  8:26         ` Iain Sandoe
  2015-03-24 20:18           ` Martin Sebor
  2015-03-24 11:51         ` Segher Boessenkool
  1 sibling, 1 reply; 9+ messages in thread
From: Iain Sandoe @ 2015-03-24  8:26 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Segher Boessenkool, Gcc Patch List, anton

Hi Martin,

On 24 Mar 2015, at 02:50, Martin Sebor wrote:

> On 03/21/2015 01:48 PM, Iain Sandoe wrote:

>>>>> 2015-03-13  Anton Blanchard  <anton@samba.org>
>>>>> 
>>>>> 	PR target/63354
>>>>> 	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): Define.
>>>>                                        ^ typo
> 
> It's fixed in version 2 of the patch posted here:
> https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00793.html

Well, I think I have the new version applied, but slip-ups are possible...

>> This ^ will cause a bootstrap fail for every rs6000 target that doesn't include linux64.h.
>> (because rs6000_keep_leaf_when_profiled will be "defined but unused").
>> 
>> Since ISTM you intend this to apply to all rs6000 sub-targets, you might as well move it to rs6000.h?
> 
> The powerpc-darwin9 and powerpc64-darwin9 targets both built
> successfully with this patch.

I'm assuming that you mean a cross-compile (which is a stage1 without -werror).  If you look in the build output (I just repeated this on x86_64-darwin12 X powerpc-darwin9) you'll see:

/GCC/gcc-trunk/gcc/config/rs6000/rs6000.c:24404:1: warning: ‘bool rs6000_keep_leaf_when_profiled()’ defined but not used [-Wunused-function]
 rs6000_keep_leaf_when_profiled (void)
 ^

this becomes a bootstrap error at stage#2.  If you don't see that, then I have don't have the same patch applied as you :).

> I also tried powerpc64-freebsd,
> which succeeded as well (though I had to work around pr65535).
> What target do you suggest I try to reproduce the failure?
> (I don't mind moving the macro definition as you suggest,
> but I'd like to understand how to trigger the problem so
> that I can reproduce it and verify that I've fixed it.)

Any target that doesn't include linux64.h would be a reproducer (AIX, powerpc-linux-gnu).
cheers,
Iain

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-24  2:50       ` Martin Sebor
  2015-03-24  8:26         ` Iain Sandoe
@ 2015-03-24 11:51         ` Segher Boessenkool
  1 sibling, 0 replies; 9+ messages in thread
From: Segher Boessenkool @ 2015-03-24 11:51 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Iain Sandoe, Gcc Patch List, anton

On Mon, Mar 23, 2015 at 08:50:27PM -0600, Martin Sebor wrote:
> >>>>	PR target/63354
> >>>>	* gcc/config/rs6000/linux64.h (ARGET_KEEP_LEAF_WHEN_PROFILED): 
> >>>>	Define.
> >>>                                        ^ typo
> 
> It's fixed in version 2 of the patch posted here:
> https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00793.html

Iain means the macro should be defined in rs6000.h, not linux64.h.

> >This ^ will cause a bootstrap fail for every rs6000 target that doesn't 
> >include linux64.h.
> >(because rs6000_keep_leaf_when_profiled will be "defined but unused").
> >
> >Since ISTM you intend this to apply to all rs6000 sub-targets, you might 
> >as well move it to rs6000.h?


Segher

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

* Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le
  2015-03-24  8:26         ` Iain Sandoe
@ 2015-03-24 20:18           ` Martin Sebor
  0 siblings, 0 replies; 9+ messages in thread
From: Martin Sebor @ 2015-03-24 20:18 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Segher Boessenkool, Gcc Patch List, anton

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

> I'm assuming that you mean a cross-compile (which is a stage1 without -werror).  If you look in the build output (I just repeated this on x86_64-darwin12 X powerpc-darwin9) you'll see:
>
> /GCC/gcc-trunk/gcc/config/rs6000/rs6000.c:24404:1: warning: ‘bool rs6000_keep_leaf_when_profiled()’ defined but not used [-Wunused-function]
>   rs6000_keep_leaf_when_profiled (void)
>   ^
>
> this becomes a bootstrap error at stage#2.  If you don't see that, then I have don't have the same patch applied as you :).

Yes, I meant a cross-compile. I don't have a Darwin environment
but I managed to reproduce it while building stage1 with -Werror
(after fixing a number of problems that cause the build to fail
earlier on). Thanks for your patience.

Attached is an updated patch with your suggested change (i.e.,
defining the TARGET_KEEP_LEAF_WHEN_PROFILED macro in rs6000.h
instead of linux64.h).

I tested the patch by building stage1 with -Werror for both
powerpc64-darwin9 and powerpc64-unknown-linux-gnu (both on
the latter target).

Martin

[-- Attachment #2: gcc-63354.patch --]
[-- Type: text/x-patch, Size: 2186 bytes --]

2015-03-24  Anton Blanchard  <anton@samba.org>

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

2015-03-24  Martin Sebor  <msebor@redhat.com>

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

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 31b46ea..f1508b9 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -24397,6 +24397,23 @@ rs6000_output_function_prologue (FILE *file,
   rs6000_pic_labelno++;
 }
 
+/* -mprofile-kernel code calls mcount before the function prologue,
+   so a profiled leaf function should stay a leaf function.  */
+
+static bool
+rs6000_keep_leaf_when_profiled (void)
+{
+  switch (DEFAULT_ABI)
+    {
+      case ABI_AIX:
+      case ABI_ELFv2:
+       return TARGET_PROFILE_KERNEL;
+
+      default:
+       return true;
+    }
+}
+
 /* 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
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index ef6bb2f..50394b0 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -703,6 +703,9 @@ extern unsigned char rs6000_recip_bits[];
 #define TARGET_CPU_CPP_BUILTINS() \
   rs6000_cpu_cpp_builtins (pfile)
 
+#undef TARGET_KEEP_LEAF_WHEN_PROFILED
+#define TARGET_KEEP_LEAF_WHEN_PROFILED rs6000_keep_leaf_when_profiled
+
 /* This is used by rs6000_cpu_cpp_builtins to indicate the byte order
    we're compiling for.  Some configurations may need to override it.  */
 #define RS6000_CPU_CPP_ENDIAN_BUILTINS()	\
diff --git a/gcc/testsuite/gcc.target/powerpc/pr63354.c b/gcc/testsuite/gcc.target/powerpc/pr63354.c
new file mode 100644
index 0000000..9e635cc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr63354.c
@@ -0,0 +1,10 @@
+/* { dg-do compile { target { powerpc64*-*-linux* } } } */
+/* { dg-options "-O2 -pg -mprofile-kernel" } */
+
+int foo (void)
+{
+  return 1;
+}
+
+/* { dg-final { scan-assembler "bl _mcount" } } */
+/* { dg-final { scan-assembler-not "(addi|stdu) 1," } } */

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

end of thread, other threads:[~2015-03-24 20:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 21:55 [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le Martin Sebor
2015-03-14 14:34 ` Segher Boessenkool
2015-03-15 23:40   ` Martin Sebor
2015-03-17 22:34     ` Segher Boessenkool
2015-03-21 19:48     ` Iain Sandoe
2015-03-24  2:50       ` Martin Sebor
2015-03-24  8:26         ` Iain Sandoe
2015-03-24 20:18           ` Martin Sebor
2015-03-24 11:51         ` 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).