Message ID | dbd35ae7e8f9ca8db89dd14b81119d3eb50afb98.1493038197.git.shorne@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 119848 invoked by alias); 24 Apr 2017 12:53:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 119832 invoked by uid 89); 24 Apr 2017 12:53:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-spam-relays-external:sk:mail-pf, H*RU:sk:mail-pf, displayed, H*r:sk:65.2017 X-HELO: mail-pf0-f196.google.com Received: from mail-pf0-f196.google.com (HELO mail-pf0-f196.google.com) (209.85.192.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Apr 2017 12:53:12 +0000 Received: by mail-pf0-f196.google.com with SMTP id v14so2588714pfd.3 for <gdb-patches@sourceware.org>; Mon, 24 Apr 2017 05:53:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=pM/SzU4LagIB/JZ8Tiz5Y2m9jiug10bml27MrgQp9l8=; b=IgwqLiGA9G6apzMiWBR0FcpFZYKZ0KF4QNtCNUE1qwaQoCvMMwZBawnxIZ7gKugxCi mj96QrJTV/UjX3Y5wIRPF36OYbuPBf/dNQM07vZdtGGYX109kHNkojQPhpcth4F9tOCb Guy4tdjgkEaGzuvz8vzq+VG82nSuRo1eLPF0LolV/oByW+0WrVrPvXr7bCeEjaKAnKf+ ZFx2MrsBtKRv2bAr0OURC6jQw3GZmooNF5ODDJQSVvHhjJ0Bz7/JLUtKTrSSdgJWqK7W wEkCHOmq7A9ax6yF+Ss4rcOoDEtV97k/J902+94autw8XTbJpMwAgqZIfrwKZnfzS6K4 ViYw== X-Gm-Message-State: AN3rC/6uXnPkk2Hofz0zfifo8vfTNvQzrCkzktbSoZPjKmBon9bzcqPX FPBwKVfPu+1waA== X-Received: by 10.84.238.195 with SMTP id l3mr33288274pln.101.1493038392620; Mon, 24 Apr 2017 05:53:12 -0700 (PDT) Received: from localhost (z64.124-44-184.ppp.wakwak.ne.jp. [124.44.184.64]) by smtp.gmail.com with ESMTPSA id s132sm13515883pgc.65.2017.04.24.05.53.11 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 24 Apr 2017 05:53:12 -0700 (PDT) From: Stafford Horne <shorne@gmail.com> To: GDB patches <gdb-patches@sourceware.org> Cc: Openrisc <openrisc@lists.librecores.org>, Franck Jullien <franck.jullien@gmail.com> Subject: [PATCH v6 1/5] tdesc: handle arbitrary strings in tdesc_register_in_reggroup_p Date: Mon, 24 Apr 2017 21:52:50 +0900 Message-Id: <dbd35ae7e8f9ca8db89dd14b81119d3eb50afb98.1493038197.git.shorne@gmail.com> In-Reply-To: <cover.1493038197.git.shorne@gmail.com> References: <cover.1493038197.git.shorne@gmail.com> In-Reply-To: <cover.1493038197.git.shorne@gmail.com> References: <cover.1493038197.git.shorne@gmail.com> X-IsSubscribed: yes |
Commit Message
Stafford Horne
April 24, 2017, 12:52 p.m. UTC
From: Franck Jullien <franck.jullien@gmail.com>
tdesc_register_in_reggroup_p in now able to handle arbitrary
groups. This is useful when groups are created while the
target descriptor file is received from the remote.
This can be the case of a soft core target processor where
registers/groups can change.
gdb/ChangeLog:
2013-02-15 Franck Jullien <franck.jullien@gmail.com>
* target-descriptions.c (tdesc_register_in_reggroup_p): Support
arbitrary strings.
---
gdb/target-descriptions.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Comments
Stafford Horne <shorne@gmail.com> writes: > tdesc_register_in_reggroup_p in now able to handle arbitrary > groups. This is useful when groups are created while the > target descriptor file is received from the remote. > > This can be the case of a soft core target processor where > registers/groups can change. > > gdb/ChangeLog: > > 2013-02-15 Franck Jullien <franck.jullien@gmail.com> > > * target-descriptions.c (tdesc_register_in_reggroup_p): Support > arbitrary strings. I am not sure what do you want to change and why do you need this change? I didn't see it in the previous version. If you want to control what registers are displayed, you can follow the way nds32 used, see nds32-tdep.c:nds32_register_reggroup_p.
On Tue, May 02, 2017 at 03:40:20PM +0100, Yao Qi wrote: > Stafford Horne <shorne@gmail.com> writes: > > > tdesc_register_in_reggroup_p in now able to handle arbitrary > > groups. This is useful when groups are created while the > > target descriptor file is received from the remote. > > > > This can be the case of a soft core target processor where > > registers/groups can change. > > > > gdb/ChangeLog: > > > > 2013-02-15 Franck Jullien <franck.jullien@gmail.com> > > > > * target-descriptions.c (tdesc_register_in_reggroup_p): Support > > arbitrary strings. > > I am not sure what do you want to change and why do you need this > change? I didn't see it in the previous version. > > If you want to control what registers are displayed, you can follow the > way nds32 used, see nds32-tdep.c:nds32_register_reggroup_p. Hi Yao, Thanks for reviewing. Sorry, this was in previous versions but dropped on accident then added back, I forgot to mention. Currenly tdesc_register_in_reggroup_p() only returns true if the register group number is one of the hard coded reggroups: float, vector, general, save or restor. This change is to also allow returning true in the case that the register group was registered with gdb with reggroup_add(). This seems like something that will be generally required so it was added to the target-descriptions definition. If you think not I can make it for openrisc only, but I think it makes more sense in tdesc. This allows the command like, the below to return the registers for the named reggroup. info reg system -Stafford
Stafford Horne <shorne@gmail.com> writes: > This change is to also allow returning true in the case that the register > group was registered with gdb with reggroup_add(). This seems like > something that will be generally required so it was added to the > target-descriptions definition. If you think not I can make it for > openrisc only, but I think it makes more sense in tdesc. Thanks for the explanation. > > This allows the command like, the below to return the registers for the > named reggroup. > > info reg system Some bits are missing, - A test case, in which we can define some reggroups "foo" and test expected "foo" is shown in the output of "maintenance print reggroups". You can add reggroups in gdb.xml/extra-regs.xml. - A test case with live inferior, get a list of support reggroups from the output of "maint print reggroups", pass each reggroup to "info reg $group", and test there is no error. - Document "info registers". https://sourceware.org/gdb/current/onlinedocs/gdb/Registers.html doesn't document the usage "info reg GROUP". We need to add it. - Add a news entry, because your patch adds a user visible change.
Stafford Horne <shorne@gmail.com> writes: > > if (reggroup == general_reggroup) > return general_p; > + > + if (strcmp (reg->group, reggroup_name (reggroup)) == 0) > + return 1; > + > } I read the patch again, and find that we can replace the whole block in "if (reg != NULL && reg->group != NULL)" with the code this patch adds, like this, if (reg != NULL && reg->group != NULL && (strcmp (reg->group, reggroup_name (reggroup)) == 0)) return 1;
On Tue, May 09, 2017 at 03:21:43PM +0100, Yao Qi wrote: > Stafford Horne <shorne@gmail.com> writes: > > > > if (reggroup == general_reggroup) > > return general_p; > > + > > + if (strcmp (reg->group, reggroup_name (reggroup)) == 0) > > + return 1; > > + > > } > > I read the patch again, and find that we can replace the whole block in > "if (reg != NULL && reg->group != NULL)" with the code this patch adds, > like this, > > if (reg != NULL && reg->group != NULL > && (strcmp (reg->group, reggroup_name (reggroup)) == 0)) > return 1; Thanks, I was thinking to simply to this as well, but I left the patch I inheherited as is. I will also update the commit log and comment to make it more clear what is going on. > -- > Yao (齐尧)
On Tue, May 09, 2017 at 02:54:01PM +0100, Yao Qi wrote: > Stafford Horne <shorne@gmail.com> writes: > > > This change is to also allow returning true in the case that the register > > group was registered with gdb with reggroup_add(). This seems like > > something that will be generally required so it was added to the > > target-descriptions definition. If you think not I can make it for > > openrisc only, but I think it makes more sense in tdesc. > > Thanks for the explanation. > > > > > This allows the command like, the below to return the registers for the > > named reggroup. > > > > info reg system > > Some bits are missing, > > - A test case, in which we can define some reggroups "foo" and test > expected "foo" is shown in the output of "maintenance print > reggroups". You can add reggroups in gdb.xml/extra-regs.xml. > > - A test case with live inferior, get a list of support reggroups from > the output of "maint print reggroups", pass each reggroup to "info > reg $group", and test there is no error. > > - Document "info registers". > https://sourceware.org/gdb/current/onlinedocs/gdb/Registers.html > doesn't document the usage "info reg GROUP". We need to add it. > > - Add a news entry, because your patch adds a user visible change. Right, I am working on this, I have most of it done. However, I am thinking to submit this separately from the OpenRISC port. There are more changes required to get this to work as you mention in the (1st) test case above. Currently tdesc supplied reggroups are not added by default, they need to be manually supplied by calling reggroup_add(). I have made a patch to make this automatic. As mentioned I will split this out separate from the OpenRISC patch and do all that is mentioned above. -Stafford
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c index 9a7e2dd..8b5ac92 100644 --- a/gdb/target-descriptions.c +++ b/gdb/target-descriptions.c @@ -1086,12 +1086,8 @@ tdesc_remote_register_number (struct gdbarch *gdbarch, int regno) return -1 if it does not know; the caller should handle registers with no specified group. - Arbitrary strings (other than "general", "float", and "vector") - from the description are not used; they cause the register to be - displayed in "info all-registers" but excluded from "info - registers" et al. The names of containing features are also not - used. This might be extended to display registers in some more - useful groupings. + The names of containing features are also not used. This might be + extended to display registers in some more useful groupings. The save-restore flag is also implemented here. */ @@ -1120,6 +1116,10 @@ tdesc_register_in_reggroup_p (struct gdbarch *gdbarch, int regno, if (reggroup == general_reggroup) return general_p; + + if (strcmp (reg->group, reggroup_name (reggroup)) == 0) + return 1; + } if (reg != NULL