From patchwork Wed Jul 11 08:59:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 28308 Received: (qmail 34235 invoked by alias); 11 Jul 2018 08:59:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 34211 invoked by uid 89); 11 Jul 2018 08:59:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=U*simon.marchi, sk:simon.m, sk:simonm, H*RU:sk:EUR03-D X-HELO: EUR03-DB5-obe.outbound.protection.outlook.com Received: from mail-eopbgr40056.outbound.protection.outlook.com (HELO EUR03-DB5-obe.outbound.protection.outlook.com) (40.107.4.56) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 Jul 2018 08:59:56 +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=WtKoCHklPcOXuzu35etxlakWaxIryDIVWmUMakZ5Q+U=; b=cp+MmVxo5mMRt2OFQDB1vKX6C7jTffr7aUzhUcYUrZCvg1Ali7w4Ky/hkBh3DAvsQp5Nue2iXQnpoGQjTZdw/qClBx5v7owztAr7nzmSGUmOy8BgdR0UvxKuRYQpMnXByizMfOo8mfsI5F690H+fteyk/If1HRsVTzJ8Zq4eum0= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.226.148) by DB6PR0802MB2536.eurprd08.prod.outlook.com (10.172.251.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.952.17; Wed, 11 Jul 2018 08:59:53 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::cdc6:80e2:3de7:782f]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::cdc6:80e2:3de7:782f%3]) with mapi id 15.20.0930.022; Wed, 11 Jul 2018 08:59:53 +0000 From: Alan Hayward To: Simon Marchi CC: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH] Return bytes for tdesc_register_size() Date: Wed, 11 Jul 2018 08:59:53 +0000 Message-ID: References: <20180710150542.93363-1-alan.hayward@arm.com> <301503d1-4575-f2fe-3acd-a0daf73dcafe@ericsson.com> In-Reply-To: <301503d1-4575-f2fe-3acd-a0daf73dcafe@ericsson.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) Content-ID: <06FE92CC03159D43A35C9333323B79B6@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-IsSubscribed: yes > On 10 Jul 2018, at 19:44, Simon Marchi wrote: > > On 2018-07-10 11:05 AM, Alan Hayward wrote: >> tdesc_register_size returns number of bits. >> Change this to use bytes, the same as regcache::register_size, memcpy and sizeof. >> >> This fixes a bug in aarch64_get_tdesc_vq which assumed bytes. >> >> Update all other calls to tdesc_register_size. >> >> I originally planned to fix aarch64_get_tdesc_vq and push as OBV. >> However, this seemed the better fix. >> Required for 8.2 >> Tested with make check on a target all build. >> I don't have a rs6000 machine, however change is simple enough. > > Hi Alan, > > Since I work with processors that have 16-bit bytes, I always prefer > expressing sizes in bits, otherwise it's ambiguous. Are we talking > about 8-bit bytes or target-sized bytes? So I would have perhaps > chosen to rename tdesc_register_size to tdesc_register_size_bits > (for clarity) and fix up the caller. > > Would you be ok with that? Other than that, the patch looks fine. > Right, that makes sense. This means regcache::register_size is at odds with that Happy with just renaming the function as it clears up the ambiguities. Pushed the following as it's obvious: diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 5120fffe776ada9fc5670ff4759c8e53378a8193..5c6eb985451c43909cc929b32eade09f778d0018 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2922,8 +2922,8 @@ aarch64_get_tdesc_vq (const struct target_desc *tdesc) if (feature_sve == nullptr) return 0; - uint64_t vl = tdesc_register_size (feature_sve, - aarch64_sve_register_names[0]); + uint64_t vl = tdesc_register_bitsize (feature_sve, + aarch64_sve_register_names[0]) / 8; return sve_vq_from_vl (vl); } diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 4eeb62ac52fcfe0c0a3648e3e68b0bb23516c73f..45bf98267a99a1c8990f7b0b0f0788dd7b0e03d3 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -5953,7 +5953,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) have_mq = tdesc_numbered_register (feature, tdesc_data, PPC_MQ_REGNUM, "mq"); - tdesc_wordsize = tdesc_register_size (feature, "pc") / 8; + tdesc_wordsize = tdesc_register_bitsize (feature, "pc") / 8; if (wordsize == -1) wordsize = tdesc_wordsize; @@ -5984,7 +5984,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) /* The fpscr register was expanded in isa 2.05 to 64 bits along with the addition of the decimal floating point facility. */ - if (tdesc_register_size (feature, "fpscr") > 32) + if (tdesc_register_bitsize (feature, "fpscr") > 32) have_dfp = 1; } else diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h index 3ba71b1add638de35c3d5bc5b6f7943a32169369..87403acc0d4d3dc7da4f2ca12c4778d5b9a5d353 100644 --- a/gdb/target-descriptions.h +++ b/gdb/target-descriptions.h @@ -125,8 +125,8 @@ int tdesc_unnumbered_register (const struct tdesc_feature *feature, /* Search FEATURE for a register named NAME, and return its size in bits. The register must exist. */ -int tdesc_register_size (const struct tdesc_feature *feature, - const char *name); +int tdesc_register_bitsize (const struct tdesc_feature *feature, + const char *name); /* Search FEATURE for a register with any of the names from NAMES (NULL-terminated). Record REGNO and the register in DATA; when diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c index 3d7aa2582e72815f98e77681fdec15acfb54ec43..a96416cd3cc94faf213cf576e9eac31524baa3ab 100644 --- a/gdb/target-descriptions.c +++ b/gdb/target-descriptions.c @@ -816,8 +816,7 @@ tdesc_numbered_register_choices (const struct tdesc_feature *feature, bits. The register must exist. */ int -tdesc_register_size (const struct tdesc_feature *feature, - const char *name) +tdesc_register_bitsize (const struct tdesc_feature *feature, const char *name) { struct tdesc_reg *reg = tdesc_find_register_early (feature, name);