From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58294 invoked by alias); 21 Mar 2015 19:48:39 -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 58283 invoked by uid 89); 21 Mar 2015 19:48:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 21 Mar 2015 19:48:37 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1YZPNn-0000gO-LZ from Iain_Sandoe@mentor.com ; Sat, 21 Mar 2015 12:48:32 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Sat, 21 Mar 2015 19:48:30 +0000 Subject: Re: [PATCH] pr 63354 - gcc -pg -mprofile-kernel creates unused stack frames on leaf functions on ppc64le MIME-Version: 1.0 (Apple Message framework v1283) Content-Type: text/plain; charset="windows-1252" From: Iain Sandoe In-Reply-To: <5506184F.8090006@redhat.com> Date: Sat, 21 Mar 2015 19:48:00 -0000 CC: Segher Boessenkool , Gcc Patch List , Content-Transfer-Encoding: quoted-printable Message-ID: <210B149C-A8AB-4A0C-98AA-90D72741A7AF@codesourcery.com> References: <55035CB1.4020801@redhat.com> <20150314143434.GC3690@gate.crashing.org> <5506184F.8090006@redhat.com> To: Martin Sebor X-SW-Source: 2015-03/txt/msg01135.txt.bz2 Hi Martin, I've applied your latest patch to top of trunk and looked at the code gen o= n powerpc-darwin9 (and a cross from x86-64-darwin12 =3D> powerpc64-linux-gn= u). 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. >>>=20 >>> 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. >>=20 >> -mprofile-kernel does nothing without profiling enabled. Maybe it >> should just have been called -pk or something horrid like that. >>=20 >> 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. >=20 > 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. >=20 >>=20 >>=20 >>> 2015-03-13 Anton Blanchard >>>=20 >>> 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 inc= lude 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? >>=20 >>> * cc/config/rs6000/rs6000.c (rs6000_keep_leaf_when_profiled). New >> ^ typo ^ ty= po >>=20 >> It shouldn't have "gcc/" in the path names at all, actually. >=20 > 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. >=20 > 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. >=20 > Martin >=20 >>=20 >>> +/* -mprofile-kernel code calls mcount before the function prolog, >>=20 >> "prologue". >>=20 >>> + so a profiled leaf function should stay a leaf function. */ >>> + >>> +static bool >>> +rs6000_keep_leaf_when_profiled (void) >>> +{ >>> + return TARGET_PROFILE_KERNEL; >>> +} >>=20 >> Something like >>=20 >> switch (DEFAULT_ABI) >> { >> case ABI_AIX: >> case ABI_ELFv2: >> return TARGET_PROFILE_KERNEL; >>=20 >> default: >> return true; >> } >>=20 >> 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 stac= k 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," } } */ >>=20 >> 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 e= xpects r#). cheers Iain >>=20 >>=20 >> Segher >>=20 >=20 >