From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22243 invoked by alias); 4 Mar 2016 14:16:31 -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 22228 invoked by uid 89); 4 Mar 2016 14:16:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=expose X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 04 Mar 2016 14:16:29 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 45B4B49; Fri, 4 Mar 2016 06:15:31 -0800 (PST) Received: from [10.2.206.200] (e100706-lin.cambridge.arm.com [10.2.206.200]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B86E93F213; Fri, 4 Mar 2016 06:16:26 -0800 (PST) Message-ID: <56D998B8.6010507@foss.arm.com> Date: Fri, 04 Mar 2016 14:16:00 -0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Nick Clifton , marcus.shawcroft@arm.com, richard.earnshaw@arm.com CC: gcc-patches@gcc.gnu.org Subject: Re: RFA: PR 70044: Catch a second call to aarch64_override_options_after_change References: <87a8metl1a.fsf@redhat.com> In-Reply-To: <87a8metl1a.fsf@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2016-03/txt/msg00358.txt.bz2 Hi Nick, On 04/03/16 13:44, Nick Clifton wrote: > Hi Markus, Hi Richard, > > PR 70044 reports a problem with the AArch64 backend. With LTO enabled > the function aarch64_override_options_after_change can be called more > than once for the same function. If only leaf frame pointers were > being omitted originally, then the first call will set > flag_omit_frame_pointer to true. Then the second call will set > flag_omit_leaf_frame_pointer to false, thus forcing the frame pointer > to always be omitted, contrary to the original settings of these > flags. > > The attached patch fixes this problem by setting > flag_omit_frame_pointer to true, but using a special value of 2 to do > so. Then when the second call occurs we can detect this case and > ensure that we do not set flag_omit_leaf_frame_pointer to false. > > Tested with no regressions on an aarch64-elf toolchain. > > OK to apply ? Thanks for looking at this. > Cheers > Nick > > gcc/ChangeLog > 2016-03-04 Nick Clifton > > PR target/7044 > * config/aarch64/aarch64.c > (aarch64_override_options_after_change_1): When forcing > flag_omit_frame_pointer to be true, use a special value that can > be detected if this function is called again, thus preventing > flag_omit_leaf_frame_pointer from being forced to be false. > + /* The logic here is that if we are disabling all frame pointer generation + then we do not need to disable leaf frame pointer generation as a + separate operation. But if we are*only* disabling leaf frame pointer + generation then we set flag_omit_frame_pointer to true, but in + aarch64_frame_pointer_required we return false only for leaf functions. + + PR 70044: We have to be careful about being called multiple times for the + same function. Once we have decided to set flag_omit_frame_pointer just + so that we can omit leaf frame pointers, we must then not interpret a + second call as meaning that all frame pointer generation should be + omitted. We do this by setting flag_omit_frame_pointer to a special, + non-zero value. */ + if (opts->x_flag_omit_frame_pointer == 2) + opts->x_flag_omit_frame_pointer = 0; + if (opts->x_flag_omit_frame_pointer) opts->x_flag_omit_leaf_frame_pointer = false; else if (opts->x_flag_omit_leaf_frame_pointer) This is missing a second hunk from the patch you attached in the PR that I think is necessary for this to work (setting to x_flag_omit_frame_pointer)... I've been testing a very similar patch that just changes that logic to: if (opts->x_flag_omit_frame_pointer == 1) opts->x_flag_omit_leaf_frame_pointer = 0; else if (opts->x_flag_omit_leaf_frame_pointer) opts->x_flag_omit_frame_pointer = 2; Note that this patch would expose a bug in common/config/aarch64/aarch64-common.c where there's a thinko in the handling of OPT_momit_leaf_frame_pointer. That's my bad and I'll propose a patch for it soon. Also, is there a way to create a testcase for the testuite? i.e. is there a simple way to scan the assembly generated after the final LTO processing for the presence of the frame pointer? Thanks, Kyrill