From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id SEKNEAi2r19/FwAAWB0awg (envelope-from ) for ; Sat, 14 Nov 2020 05:48:40 -0500 Received: by simark.ca (Postfix, from userid 112) id 41CC91F08B; Sat, 14 Nov 2020 05:48:40 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 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 EBA341E58F for ; Sat, 14 Nov 2020 05:48:39 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8A72F385702E; Sat, 14 Nov 2020 10:48:39 +0000 (GMT) Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTP id 2B2183858001 for ; Sat, 14 Nov 2020 10:48:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2B2183858001 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E152D56019; Sat, 14 Nov 2020 05:48:35 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id G3VygCi7pE5w; Sat, 14 Nov 2020 05:48:35 -0500 (EST) Received: from float.home (localhost.localdomain [127.0.0.1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id 8209256018; Sat, 14 Nov 2020 05:48:35 -0500 (EST) Received: by float.home (Postfix, from userid 1000) id 02983A1871; Sat, 14 Nov 2020 14:48:29 +0400 (+04) Date: Sat, 14 Nov 2020 14:48:29 +0400 From: Joel Brobecker To: Simon Marchi Subject: Re: [PATCH 5/9] Add support for printing value of DWARF-based fixed-point type objects Message-ID: <20201114104829.GA404828@adacore.com> References: <1604817017-25807-1-git-send-email-brobecker@adacore.com> <1604817017-25807-6-git-send-email-brobecker@adacore.com> <2d12525a-3a85-3f69-bfea-22166f7fd358@simark.ca> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="7AUc2qLy4jB3hD7Z" Content-Disposition: inline In-Reply-To: <2d12525a-3a85-3f69-bfea-22166f7fd358@simark.ca> 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: , Cc: GDB patches Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" --7AUc2qLy4jB3hD7Z Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Simon, > > + of the corresponding TYPE by setting its TYPE_FIXED_POINT_INFO. > > + CU is the DIE's CU. */ > > + > > +static void > > +finish_fixed_point_type (struct type *type, struct die_info *die, > > + struct dwarf2_cu *cu) > > Unless there's a good reason not to (coming up in the following > patches), I would make this function create the "struct type", instead > of having the caller create it and pass it. In other words, this > signature: > > struct type *create_fixed_point_type (die_info *die, dwarf2_cu *cu) > > That just makes it more obvious that it's a simple "die" to "type" > transform. While implementing this suggestion, I'm realizing that doing so requires either: (1) Recomputing the following information for the CU: encoding, bitsize if any, and type name, which means: | attr = dwarf2_attr (die, DW_AT_encoding, cu); | if (attr != nullptr && attr->form_is_constant ()) | encoding = attr->constant_value (0); | attr = dwarf2_attr (die, DW_AT_byte_size, cu); | if (attr != nullptr) | bits = attr->constant_value (0) * TARGET_CHAR_BIT; | name = dwarf2_name (die, cu); | if (!name) | complaint (_("DW_AT_name missing from DW_TAG_base_type")); Or: (2) Passing that information as arguments to the function. I find option (1) really sad because of the code duplication. And I find option (2) somewhat unsatisfactory, because we then run the risk of passing inconsistent information. This might explain partly why the init + finish approach isn't new in this unit... I'm attaching the patch which shows how option (1) looks like. For me, the current approach strikes a better balance between API cleanliness and implementation considerations. But I don't have a strong opinion against implementing what you suggest, including through option (2) (or other options I missed). Let me know what you think. -- Joel --7AUc2qLy4jB3hD7Z Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-make-finish_fixed_point_type-create-the-type-renamin.patch" >From 2e76b9ca55aa93189d22d649c8698ab8d572ed7a Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Sat, 14 Nov 2020 05:43:33 -0500 Subject: [PATCH] make finish_fixed_point_type create the type (renaming it accordingly) Change-Id: Ic1b81f5e8328617627675a609ede8320edd749c0 --- gdb/dwarf2/read.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 4a447d4..6ba24b3 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -18221,14 +18221,17 @@ get_dwarf2_unsigned_rational_constant (struct die_info *die, of the corresponding TYPE by setting its TYPE_FIXED_POINT_INFO. CU is the DIE's CU. */ -static void -finish_fixed_point_type (struct type *type, struct die_info *die, - struct dwarf2_cu *cu) +static struct type * +read_fixed_point_type (struct die_info *die, struct dwarf2_cu *cu) { struct attribute *attr; + int encoding = 0; - gdb_assert (type->code () == TYPE_CODE_FIXED_POINT - && TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_FIXED_POINT); + attr = dwarf2_attr (die, DW_AT_encoding, cu); + if (attr != nullptr && attr->form_is_constant ()) + encoding = attr->constant_value (0); + gdb_assert (encoding == DW_ATE_signed_fixed + || encoding == DW_ATE_unsigned_fixed); attr = dwarf2_attr (die, DW_AT_binary_scale, cu); if (!attr) @@ -18289,10 +18292,25 @@ finish_fixed_point_type (struct type *type, struct die_info *die, sect_offset_str (die->sect_off)); } + int bits = 0; + + attr = dwarf2_attr (die, DW_AT_byte_size, cu); + if (attr != nullptr) + bits = attr->constant_value (0) * TARGET_CHAR_BIT; + const char *name = dwarf2_name (die, cu); + if (!name) + complaint (_("DW_AT_name missing from DW_TAG_base_type")); + + struct type *type = init_fixed_point_type + (cu->per_objfile->objfile, bits, encoding == DW_ATE_unsigned_fixed, + name); + gdb_mpq &scaling_factor = TYPE_FIXED_POINT_INFO (type)->scaling_factor; mpz_set (mpq_numref (scaling_factor.val), scale_num.val); mpz_set (mpq_denref (scaling_factor.val), scale_denom.val); mpq_canonicalize (scaling_factor.val); + + return type; } /* Allocate a floating-point type of size BITS and name NAME. Pass NAME_HINT @@ -18574,12 +18592,8 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu) } break; case DW_ATE_signed_fixed: - type = init_fixed_point_type (objfile, bits, 0, name); - finish_fixed_point_type (type, die, cu); - break; case DW_ATE_unsigned_fixed: - type = init_fixed_point_type (objfile, bits, 1, name); - finish_fixed_point_type (type, die, cu); + type = read_fixed_point_type (die, cu); break; default: -- 2.1.4 --7AUc2qLy4jB3hD7Z--