From patchwork Tue Mar 25 08:51:27 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcus Shawcroft X-Patchwork-Id: 333330 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B5D6B14008C for ; Tue, 25 Mar 2014 19:51:40 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=gM7Dgt8nYdyjsf3ldfyl5+NS+ln/Bli3gAneoKI9ytu9UB q4W/cSsHCbO5j1IIdVLJQi71SuEHsCsvUsJWHGWTj2jGzOcLosWfPocB6dBrA1aR GfHwA6yOO1CeYBJuxYmPnmY9CiAsvj85plJ6oGUFfT8inS4Ni5Fcoq5ir/j8s= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=IBLR91U0UPVSuiKBU4Dhh25oatM=; b=s4w6ss38NJ7auMK2mKCz zBTHidOUr3uEa0YmADu1NAVCeSLjP8JsBTxcvrNk78l3PRS9YQ9Izw1sqO367Ofe xBiUxFeuwdYfJLQmpsKUC7cXujI1Sy7OfCn+lfGWPonIBVklAIGKWnZsvdkATOyI MyZ/xMm5GB1PiSccCtzWZ9o= Received: (qmail 4727 invoked by alias); 25 Mar 2014 08:51:33 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 4717 invoked by uid 89); 25 Mar 2014 08:51:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Mar 2014 08:51:31 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Tue, 25 Mar 2014 08:51:28 +0000 Received: from [10.1.207.52] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 25 Mar 2014 08:51:42 +0000 Message-ID: <5331438F.5060605@arm.com> Date: Tue, 25 Mar 2014 08:51:27 +0000 From: Marcus Shawcroft User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" Subject: [PATCH, PR 60580, AArch64] Fix __attribute__ ((optimize("no-omit-frame-pointer"))) X-MC-Unique: 114032508512804701 Hi, The implementation of -m[no-]omit-leaf-frame-pointer and -f[no-]omit-frame-pointer in the AArch64 target does not behave correctly in the presence of __attribute__ optimize. This patch adjusts the implementation to work in a similar fashion to the same functionality in the i386 target. The problem occurs because the current implementation uses a global 'faked_omit_frame_pointer' to retain the original value of flag_omit_frame_pointer. The global does not form part of the optimization save state. This solution removes the global and instead tracks required behaviour using only flag_omit_frame_pointer and flag_omit_leaf_frame_pointer. These two form part of the optimziation save state and target save state respectively. The additional complication for AArch64 is that the PCS requires that given -fno-omit-frame-pointer -momit-leave-frame-pointer, a leaf function that kills LR must create a frame record. This is readily handled in aarch64_frame_pointer_required(). I've dropped logic in aarch64_can_eliminate() that attempts to detect this scenario since it appears to me to be superflous. I'll leave this on the list for 24h for folks to comment and to give the RM's the chance to object before committing. Cheers /Marcus 2014-03-25 Marcus Shawcroft PR target/60580 * config/aarch64/aarch64.c (faked_omit_frame_pointer): Remove. (aarch64_frame_pointer_required): Adjust logic. (aarch64_can_eliminate): Adjust logic. (aarch64_override_options_after_change): Adjust logic. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index d59ecf7..7bd608c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2014-03-21 Marcus Shawcroft + + PR target/60580 + * config/aarch64/aarch64.c (faked_omit_frame_pointer): Remove. + (aarch64_frame_pointer_required): Adjust logic. + (aarch64_can_eliminate): Adjust logic. + (aarch64_override_options_after_change): Adjust logic. + 2014-03-19 Venkataramanan Kumar * config/aarch64/aarch64.md (stack_protect_set, stack_protect_test) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index a0b20b67..c360289 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -315,10 +315,6 @@ static GTY(()) int gty_dummy; #define AARCH64_NUM_BITMASKS 5334 static unsigned HOST_WIDE_INT aarch64_bitmasks[AARCH64_NUM_BITMASKS]; -/* Did we set flag_omit_frame_pointer just so - aarch64_frame_pointer_required would be called? */ -static bool faked_omit_frame_pointer; - typedef enum aarch64_cond_code { AARCH64_EQ = 0, AARCH64_NE, AARCH64_CS, AARCH64_CC, AARCH64_MI, AARCH64_PL, @@ -1730,17 +1726,15 @@ aarch64_frame_pointer_required (void) if (cfun->calls_alloca) return true; - /* We may have turned flag_omit_frame_pointer on in order to have this - function called; if we did, we also set the 'faked_omit_frame_pointer' flag - and we'll check it here. - If we really did set flag_omit_frame_pointer normally, then we return false - (no frame pointer required) in all cases. */ + /* In aarch64_override_options_after_change + flag_omit_leaf_frame_pointer turns off the frame pointer by + default. Turn it back on now if we've not got a leaf + function. */ + if (flag_omit_leaf_frame_pointer + && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM))) + return true; - if (flag_omit_frame_pointer && !faked_omit_frame_pointer) - return false; - else if (flag_omit_leaf_frame_pointer) - return !crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM); - return true; + return false; } /* Mark the registers that need to be saved by the callee and calculate @@ -4168,23 +4162,8 @@ aarch64_can_eliminate (const int from, const int to) return true; if (from == FRAME_POINTER_REGNUM && to == HARD_FRAME_POINTER_REGNUM) return true; - return false; - } - else - { - /* If we decided that we didn't need a leaf frame pointer but then used - LR in the function, then we'll want a frame pointer after all, so - prevent this elimination to ensure a frame pointer is used. - - NOTE: the original value of flag_omit_frame_pointer gets trashed - IFF flag_omit_leaf_frame_pointer is true, so we check the value - of faked_omit_frame_pointer here (which is true when we always - wish to keep non-leaf frame pointers but only wish to keep leaf frame - pointers when LR is clobbered). */ - if (to == STACK_POINTER_REGNUM - && df_regs_ever_live_p (LR_REGNUM) - && faked_omit_frame_pointer) - return false; + + return false; } return true; @@ -5314,17 +5293,10 @@ aarch64_override_options (void) static void aarch64_override_options_after_change (void) { - faked_omit_frame_pointer = false; - - /* To omit leaf frame pointers, we need to turn flag_omit_frame_pointer on so - that aarch64_frame_pointer_required will be called. We need to remember - whether flag_omit_frame_pointer was turned on normally or just faked. */ - - if (flag_omit_leaf_frame_pointer && !flag_omit_frame_pointer) - { - flag_omit_frame_pointer = true; - faked_omit_frame_pointer = true; - } + if (flag_omit_frame_pointer) + flag_omit_leaf_frame_pointer = false; + else if (flag_omit_leaf_frame_pointer) + flag_omit_frame_pointer = true; } static struct machine_function * diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 8b3c9b1..d97ed95 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2014-03-21 Marcus Shawcroft + + PR target/60580 + * gcc.target/aarch64/pr60580_1.c: New. + * gcc.target/aarch64/test_fp_attribute_1.c: New. + * gcc.target/aarch64/test_fp_attribute_2.c: New. + 2014-03-14 Alex Velenko * gcc.target/aarch64/vdup_lane_1.c: New testcase. diff --git a/gcc/testsuite/gcc.target/aarch64/pr60580_1.c b/gcc/testsuite/gcc.target/aarch64/pr60580_1.c new file mode 100644 index 0000000..1adf508 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr60580_1.c @@ -0,0 +1,45 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -fomit-frame-pointer -fno-inline --save-temps" } */ + +void +func_leaf (void) +{ + int a = 0; +} + +void +func_no_leaf (void) +{ + int a = 0; + func_leaf (); +} + +void +func1 (void) +{ + int a = 0; + func_no_leaf (); +} + +/* + * This function calls XXX(), which modifies SP. This is incompatible to + * -fomit-frame-pointer generated code as SP is used to access the frame. + */ +__attribute__ ((optimize("no-omit-frame-pointer"))) +void +func2 (void) +{ + int a = 0; + func_no_leaf (); +} + +void +func3 (void) +{ + int a = 0; + func_no_leaf (); +} + +/* { dg-final { scan-assembler-times "stp\tx29, x30, \\\[sp, -\[0-9\]+\\\]!" 1 } } */ + +/* { dg-final { cleanup-saved-temps } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/test_fp_attribute_1.c b/gcc/testsuite/gcc.target/aarch64/test_fp_attribute_1.c new file mode 100644 index 0000000..7538250 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/test_fp_attribute_1.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -fno-omit-frame-pointer -fno-inline --save-temps" } */ + +void +leaf (void) +{ + int a = 0; +} + +__attribute__ ((optimize("omit-frame-pointer"))) +void +non_leaf_1 (void) +{ + leaf (); +} + +__attribute__ ((optimize("omit-frame-pointer"))) +void +non_leaf_2 (void) +{ + leaf (); +} + +/* { dg-final { scan-assembler-times "str\tx30, \\\[sp\\\]" 2 } } */ + +/* { dg-final { cleanup-saved-temps } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/test_fp_attribute_2.c b/gcc/testsuite/gcc.target/aarch64/test_fp_attribute_2.c new file mode 100644 index 0000000..675091f --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/test_fp_attribute_2.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -fomit-frame-pointer -fno-inline --save-temps" } */ + +void +leaf (void) +{ + int a = 0; +} + +__attribute__ ((optimize("no-omit-frame-pointer"))) +void +non_leaf_1 (void) +{ + leaf (); +} + +__attribute__ ((optimize("no-omit-frame-pointer"))) +void +non_leaf_2 (void) +{ + leaf (); +} + +/* { dg-final { scan-assembler-times "stp\tx29, x30, \\\[sp, -\[0-9\]+\\\]!" 2 } } */ + +/* { dg-final { cleanup-saved-temps } } */