From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23342 invoked by alias); 15 Mar 2015 23:40:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 23329 invoked by uid 89); 15 Mar 2015 23:40:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sun, 15 Mar 2015 23:40:05 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2FNe2rT011322 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Sun, 15 Mar 2015 19:40:03 -0400 Received: from [10.10.52.72] (unused [10.10.52.72] (may be forged)) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2FNe0rJ002394; Sun, 15 Mar 2015 19:40:00 -0400 Message-ID: <5506184F.8090006@redhat.com> Date: Sun, 15 Mar 2015 23:40:00 -0000 From: Martin Sebor User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Segher Boessenkool CC: Gcc Patch List , anton@samba.org Subject: Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le References: <55035CB1.4020801@redhat.com> <20150314143434.GC3690@gate.crashing.org> In-Reply-To: <20150314143434.GC3690@gate.crashing.org> Content-Type: multipart/mixed; boundary="------------090208020104060201070002" X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00793.txt.bz2 This is a multi-part message in MIME format. --------------090208020104060201070002 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3256 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 >> >> 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 > --------------090208020104060201070002 Content-Type: text/x-patch; name="gcc-63354.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-63354.patch" Content-length: 2040 2015-03-13 Anton Blanchard 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 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," } } */ --------------090208020104060201070002--