From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by sourceware.org (Postfix) with ESMTPS id 36C433858D28 for ; Wed, 25 Jan 2023 20:16:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 36C433858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=foss.st.com Received: from pps.filterd (m0288072.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 30PHTc23010489; Wed, 25 Jan 2023 21:16:02 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=9UuOAligZVSjeWRLVROa6gpcAw30LnG4Saj1OUDwwoA=; b=FnciL3EGzL7qax8kfNSw+axIlSEhluYUAU7BDtumCYNfcdIi+n4ag3jvDtz093sopWyD a0rg1w18jWtZYNvEXe5kB/v6xrbihoRpSgl7ZEWpscmRYysNmTY6FUEWxf/gk82Jriyo pHY5PiXkkugWzG+qV/qygx1cHRJpOtSnVa3rJJDPKYGRWd9TL6J+kVzg4xXCsKxkJej3 z4jfu3HWHQ3PFgwQGk3DtuQxD74fWzE3BS388ycHraiwY7kpMIO+ITP1NmNy+vFSpJSV ZRSAsH7FG7YlZMODImOywf08dN3uYJ2cGDuUZ4sXolXK87uqJEWIKYmPad2D4Dc2Vn8N ZQ== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3n89eq0ff0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 25 Jan 2023 21:16:02 +0100 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id E884D10002A; Wed, 25 Jan 2023 21:16:00 +0100 (CET) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id DC36A22D17A; Wed, 25 Jan 2023 21:16:00 +0100 (CET) Received: from [10.252.16.216] (10.252.16.216) by SHFDAG1NODE3.st.com (10.75.129.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.13; Wed, 25 Jan 2023 21:15:59 +0100 Message-ID: Date: Wed, 25 Jan 2023 21:15:58 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v3 2/2] gdb/arm: Use new dwarf2 function cache Content-Language: en-US To: Simon Marchi , Luis Machado , CC: , Yvan Roux References: <20230119102948.3069226-1-torbjorn.svensson@foss.st.com> <20230119102948.3069226-2-torbjorn.svensson@foss.st.com> <801f740d-9186-52dc-5bd4-e3416f001c17@arm.com> <4a2b3e01-570c-65e7-544f-924f0ff7576b@simark.ca> From: Torbjorn SVENSSON In-Reply-To: <4a2b3e01-570c-65e7-544f-924f0ff7576b@simark.ca> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.252.16.216] X-ClientProxiedBy: EQNCAS1NODE3.st.com (10.75.129.80) To SHFDAG1NODE3.st.com (10.75.129.71) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1 definitions=2023-01-25_13,2023-01-25_01,2022-06-22_01 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-01-25 18:12, Simon Marchi wrote: > > > On 1/25/23 11:55, Luis Machado via Gdb-patches wrote: >> On 1/19/23 10:29, Torbjörn SVENSSON wrote: >>> v2 -> v3: >>> No changes, just rebase. >>> >>> --- >>> >>> This patch resolves the performance issue reported in pr/29738 by >>> caching the values for the stack pointers for the inner frame. By >>> doing so, the impact can be reduced to checking the state and >>> returning the appropriate value. >>> >>> Signed-off-by: Torbjörn SVENSSON >>> Signed-off-by: Yvan Roux >>> --- >>> gdb/arm-tdep.c | 96 +++++++++++++++++++++++++++++++++----------------- >>> 1 file changed, 64 insertions(+), 32 deletions(-) >>> >>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c >>> index 51ec5236af1..be7219ca66e 100644 >>> --- a/gdb/arm-tdep.c >>> +++ b/gdb/arm-tdep.c >>> @@ -3964,6 +3964,18 @@ struct frame_base arm_normal_base = { >>> arm_normal_frame_base >>> }; >>> +struct arm_dwarf2_prev_register_cache >>> +{ >>> + /* Cached value of the coresponding stack pointer for the inner frame. */ >>> + CORE_ADDR sp; >>> + CORE_ADDR msp; >>> + CORE_ADDR msp_s; >>> + CORE_ADDR msp_ns; >>> + CORE_ADDR psp; >>> + CORE_ADDR psp_s; >>> + CORE_ADDR psp_ns; >>> +}; >>> + >>> static struct value * >>> arm_dwarf2_prev_register (frame_info_ptr this_frame, void **this_cache, >>> int regnum) >>> @@ -3972,6 +3984,48 @@ arm_dwarf2_prev_register (frame_info_ptr this_frame, void **this_cache, >>> arm_gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); >>> CORE_ADDR lr; >>> ULONGEST cpsr; >>> + struct arm_dwarf2_prev_register_cache *cache >>> + = (struct arm_dwarf2_prev_register_cache *) dwarf2_frame_get_fn_data ( >>> + this_frame, this_cache, arm_dwarf2_prev_register); >>> + >>> + if (!cache) >>> + { >>> + const unsigned int size = sizeof (struct arm_dwarf2_prev_register_cache); >>> + cache = (struct arm_dwarf2_prev_register_cache *) >>> + dwarf2_frame_allocate_fn_data (this_frame, this_cache, >> >> Still some funny spacing above. Could you please check the patch again, just to be sure? > > To clarify, I would probably write the first part as: > > arm_dwarf2_prev_register_cache *cache > = ((arm_dwarf2_prev_register_cache *) > dwarf2_frame_get_fn_data (this_frame, this_cache, > arm_dwarf2_prev_register)); > > > The changes are: > > - dropping the struct keyword (unnecessary in C++) > - the assigment operator is on the second line (mandated by GNU style) > - the addition of parenthesis when breaking an expression (also > mandated by GNU style, is it so that the following line(s) align > nicely with the opening parenthesis) > > Similarly, the second one would be: > > cache = ((arm_dwarf2_prev_register_cache *) > dwarf2_frame_allocate_fn_data (this_frame, this_cache, > arm_dwarf2_prev_register, size); > > This is just how I would do it, there are probably other valid ways. > > Simon I pushed the patchset with the above changes to trunk. Thanks for the review of these! Kind regards, Torbjörn