From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id zDtLGAdsrGPMzgoAWB0awg (envelope-from ) for ; Wed, 28 Dec 2022 11:17:11 -0500 Received: by simark.ca (Postfix, from userid 112) id 54B7F1E222; Wed, 28 Dec 2022 11:17:11 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=tgfBdTf0; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_HI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3BDA71E0D3 for ; Wed, 28 Dec 2022 11:17:10 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9D51F3858C66 for ; Wed, 28 Dec 2022 16:17:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9D51F3858C66 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1672244229; bh=vOpCkln5qikuRhVxVQ39/H/5eSuCa+W55N6VtEucwX8=; h=Date:Subject:To:CC:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=tgfBdTf0iFug458aloADDUtPaPc0h2JLvvDPLaZecBt3HEss2RIaCwo6S9gqMV7aE rFartlEG19aDAYy3gxHJcbyWNdySr+EXBn5+vAfPpHAfcc2Z2G4INqZ80guSd60qFp 4rsTiFBu6rfBr7/JgtYpcjoJ+Cdmvp4/JnaO+8mM= Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) by sourceware.org (Postfix) with ESMTPS id 3EF593858D33 for ; Wed, 28 Dec 2022 16:16:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3EF593858D33 Received: from pps.filterd (m0046661.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2BS9bRoX008835; Wed, 28 Dec 2022 17:16:47 +0100 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 3mns6pjkmx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 28 Dec 2022 17:16:47 +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 24408100038; Wed, 28 Dec 2022 17:16:46 +0100 (CET) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id D4B1B2278A1; Wed, 28 Dec 2022 17:16:05 +0100 (CET) Received: from [10.252.10.162] (10.252.10.162) 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, 28 Dec 2022 17:16:04 +0100 Message-ID: Date: Wed, 28 Dec 2022 17:16:03 +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 v2 3/4] gdb: dwarf2 generic implementation for caching function data To: Tom Tromey , =?UTF-8?Q?Torbj=c3=b6rn_SVENSSON_via_Gdb-patches?= CC: , , Yvan Roux References: <20221118155252.113476-1-torbjorn.svensson@foss.st.com> <20221118155252.113476-4-torbjorn.svensson@foss.st.com> <87ili5kcu6.fsf@tromey.com> Content-Language: en-US In-Reply-To: <87ili5kcu6.fsf@tromey.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.252.10.162] X-ClientProxiedBy: EQNCAS1NODE4.st.com (10.75.129.82) To SHFDAG1NODE3.st.com (10.75.129.71) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-12-28_13,2022-12-28_02,2022-06-22_01 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Torbjorn SVENSSON via Gdb-patches Reply-To: Torbjorn SVENSSON Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2022-12-20 22:02, Tom Tromey wrote: >>>>>> Torbjörn SVENSSON via Gdb-patches writes: > >> When there is no dwarf2 data for a register, a function can be called >> to provide the value of this register. In some situations, it might >> not be trivial to determine the value to return and it would cause a >> performance bottleneck to do the computation each time. > > Thanks for the patch. > >> This patch allows the called function to have a "cache" object that it >> can use to store some metadata between calls to reduce the performance >> impact of the complex logic. > >> The cache object is unique for each function and frame, so if there are >> more than one function pointer stored in the dwarf2_frame_cache->reg >> array, then the appropriate pointer will be supplied (the type is not >> known by the dwarf2 implementation). > > Does this ever happen? If not perhaps a simpler approach would be > better. Right now; I don't know, but as the fn member in the dwarf2_frame_state_reg struct contains one function pointer per register, the architecture allows more than one function pointer per frame. If we went with a simpler solution, to only have one data block per frame, regardless of what function that is "owning" the data, then it could lead to nasty surprises if there is some unwinder that expects to be able to use more than data type... If we move the function pointer from the register scope to the frame scope, then I agree that only one data object would be needed. If we stick to having the function pointer per register, I could accept to have one data block, but somewhere, an assert should added so that the wrongful assumption mentioned above would be caught early rather than leading to strange bugs. This would mean that the type needs to be stored in the dwarf2_frame_cache struct somehow, but the type is currently internal to another compile unit. This is basically the reason why I went with the decoupled solution that is provided in this patch. > >> +struct dwarf2_frame_fn_data >> +{ > > New type should have a comment. Okay, I'll add comments, but I would like to know if this is the way to go or if there should be some alternative implementation before spending more time on this. > >> + struct value *(*fn) (frame_info_ptr this_frame, void **this_cache, >> + int regnum); > > Shouldn't this use the fn_prev_register typedef? Indeed. > >> + void *data; >> + struct dwarf2_frame_fn_data* next; > > Wrong placement of the "*", but really a lot of the code isn't following > the GNU / gdb style. Don't know why the contrib/check_GNU_style.py in the GCC source tree did not flag this. Anyway, will be fixed in v3. >> +void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame, void **this_cache, >> + fn_prev_register fn) > > Normally new functions get a comment referring to the header where they > are declared. Can you point me to an example and I will do something similar for these new functions if we decide to go this way. >> + >> +/* Allocate a new instance of the function unique data. */ >> + >> +extern void *dwarf2_frame_allocate_fn_data (frame_info_ptr this_frame, >> + void **this_cache, >> + fn_prev_register fn, >> + unsigned long size); >> + >> +/* Retrieve the function unique data for this frame. */ >> + >> +extern void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame, > > I think these comments could perhaps be expanded a bit. What more detail would you like to include? > > Tom Kind regards, Torbjörn