From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40201 invoked by alias); 12 Sep 2018 13:07:50 -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 39732 invoked by uid 89); 12 Sep 2018 13:07:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: EUR01-VE1-obe.outbound.protection.outlook.com Received: from mail-ve1eur01on0052.outbound.protection.outlook.com (HELO EUR01-VE1-obe.outbound.protection.outlook.com) (104.47.1.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Sep 2018 13:07:47 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=aKyhE143Diyy86QJT2y8kGrKZcxvN2UcYxCXZhMujiU=; b=YWGrL2++wpxBRn+uKJmAe/fhy+gqiNFx0N1UnHIXXtJNmy5lGO++5WlYnAA22p62xTF9ybI6q7Hl3ZFVasML20vEr1QiMZCVu3JJuNPTx4hfzYMKJjDW6mf1FshRHXhuAbil5BeZ4akSNeadwL0Y/Oi1jITb4oHQXj8HS9F1Rc0= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Vlad.Lazar@arm.com; Received: from [10.2.206.48] (217.140.106.50) by HE1PR0801MB1644.eurprd08.prod.outlook.com (2603:10a6:3:86::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1122.18; Wed, 12 Sep 2018 13:07:43 +0000 Subject: Re: [PATCH][AAarch64][v3] Add support for TARGET_COMPUTE_FRAME_LAYOUT To: James Greenhalgh References: <0f11a4e3-9c67-e9b6-49dc-4b3573ca8f0f@arm.com> <20180911165352.GC37978@arm.com> Cc: "gcc-patches@gcc.gnu.org" , nd From: Vlad Lazar Message-ID: <5B990F9D.6000304@arm.com> Date: Wed, 12 Sep 2018 13:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <20180911165352.GC37978@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-Path: vlad.lazar@arm.com Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00609.txt.bz2 On 11/09/18 17:53, James Greenhalgh wrote: > On Mon, Aug 06, 2018 at 11:14:17AM -0500, Vlad Lazar wrote: >> Hi, >> >> The patch adds support for the TARGET_COMPUTE_FRAME_LAYOUT hook on AArch64 >> and removes unneeded frame layout recalculation. >> >> The removed aarch64_layout_frame calls are unnecessary because the functions in which >> they appear will be called during or after the reload pass in which the TARGET_COMPUTE_FRAME_LAYOUT >> hook is called. The if statement in aarch64_layout_frame had the purpose of avoiding >> the extra work from the calls which have been removed and is now redundant. > > I'm not sure I understand, I may be missing something as the frame layout > is complex, but I can't get where I need to be to accept your patch from this > comment. > > The check you removed ensures that if we're after reload, and the frame is > laid out, we do no additional work. That part I understand, and that would > mean that any post-reload calls were no-ops. Is the argument that all > users of this code that you eliminate are after reload, and consequently > would have hit this no-op path? Can you talk me through why each case is > safe? > Thanks for taking a look at the patch. Indeed, all the removed calls are happening during or after reload. I'll go trough all of them and try to explain the rationale behind. aarch64_expand_prologue and aarch64_expand_epilogue are called after the pro_and_epilogue pass, which runs after reload where TARGET_COMPUTE_FRAME_LAYOUT is called. aarch64_use_return_insn_p checks explicitly for reload_completed at the beginning of the function and returns false if reload has not run. So it's safe to remove the call as the frame layout is computed by the time it reaches that point. aarch64_get_separate_components implements the TARGET_SHRINK_WRAP_GET_SEPARATE_COMPONENTS hook. This hook only seems to be used int shrink_wrap.c:try_shrink_wrapping_separate. The actual origin of this hook call can be traced back to the pro_and_epilogue pass: shrink_wrap.c:try_shrink_wrapping_separate <- function.c:thread_prologue_and_epilogue_insns <- function.c:rest_of_handle_thread_prologue_and_epilogue (pro_and_epilogue pass entry point). Therefore, aarch64_get_separate_components only gets called post reload. aarch64_get_separate_components implements the INITIAL_ELIMINATION_OFFSET hook, which is used in: 1. rtlanal.c:get_initial_register_offset: Before using the hook it checks that reload has been completed. 2. reload1.c:get_initial_register_offset and reload1.c:set_initial_elim_offsets: These functions explicitly call TARGET_COMPUTE_FRAME_LAYOUT before using the hook. 3. lra-eliminitations.c:update_reg_eliminate: The TARGET_COMPUTE_FRAME_LAYOUT is, again, called before the INITIAL_ELIMINATION_OFFSET hook is used. I hope this helps make things a bit clearer. Thanks, Vlad > Thanks, > James > >> gcc/ >> 2018-08-06 Vlad Lazar >> >> * config/aarch64/aarch64.h (TARGET_COMPUTE_FRAME_LAYOUT): Define. >> * config/aarch64/aarch64.c (aarch64_expand_prologue): Remove aarch64_layout_frame call. >> (aarch64_expand_epilogue): Likewise. >> (aarch64_initial_elimination_offset): Likewise. >> (aarch64_get_separate_components): Likewise. >> (aarch64_use_return_insn_p): Likewise. >> (aarch64_layout_frame): Remove unneeded check.