* [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).