Message ID | D46B0D70.154DB%alan.hayward@arm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 11128 invoked by alias); 5 Dec 2016 12:26:31 -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 11117 invoked by uid 89); 5 Dec 2016 12:26:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=transferring, ptid, H*u:sk:Microso, H*UA:sk:Microso X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 05 Dec 2016 12:26:29 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6F54415AB; Mon, 5 Dec 2016 04:26:28 -0800 (PST) Received: from [10.45.32.207] (e105284-mac.manchester.arm.com [10.45.32.207]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 983FF3F445 for <gdb-patches@sourceware.org>; Mon, 5 Dec 2016 04:26:27 -0800 (PST) User-Agent: Microsoft-MacOutlook/14.7.0.161029 Date: Mon, 05 Dec 2016 12:26:24 +0000 Subject: [PATCH 1/8] AARCH64 SVE: Increse max register sizes From: Alan Hayward <alan.hayward@arm.com> To: <gdb-patches@sourceware.org> Message-ID: <D46B0D70.154DB%alan.hayward@arm.com> Mime-version: 1.0 Content-type: text/plain; charset="UTF-8" Content-transfer-encoding: 7bit |
Commit Message
Alan Hayward
Dec. 5, 2016, 12:26 p.m. UTC
This is part of a series adding AARCH64 SVE support to gdb and gdbserver. In SVE the maximum size of a variable-length vector register is 256 bytes, four times the current maximum size currently supported in gdb. This patch increases the max register size and max gdbserver buffer size accordingly. Alternatively, I could add a target variable using gdbarch.c, however there are 80+ static arrays within the code using the value, which would all need replacing with mallocs/frees. Tested on x86 and aarch64. Ok to commit as is? Alan. /* Definition for an unknown syscall, used basically in error-cases. */ #define UNKNOWN_SYSCALL (-1)
Comments
On 16-12-05 12:26:24, Alan Hayward wrote: > This is part of a series adding AARCH64 SVE support to gdb and gdbserver. > > In SVE the maximum size of a variable-length vector register is 256 bytes, > four > times the current maximum size currently supported in gdb. This patch > increases > the max register size and max gdbserver buffer size accordingly. Joel expressed the willingness that we should make MAX_REGISTER_SIZE gdbarch specific last time when it was changed from 32 to 64. https://sourceware.org/ml/gdb-patches/2010-09/msg00245.html I think we should make MAX_REGISTER_SIZE gdbarch specific, or stop using it at all. > > Alternatively, I could add a target variable using gdbarch.c, however > there are > 80+ static arrays within the code using the value, which would all need > replacing with mallocs/frees. We can use alloca to allocate memory on stack, and we can get the size of a register if gdbarch and regnum is available. For example, we can replace MAX_REGISTER_SIZE with register_size in xtensa_pseudo_register_read this way, gdb_byte *buf = (gdb_byte *) alloca (register_size (gdbarch, regnum)); This can be used many places to replace MAX_REGISTER_SIZE. Hopefully, this may get rid of the use of MAX_REGISTER_SIZE except the uses in python/py-unwind.c and remote.c. We may replace array data[MAX_REGISTER_SIZE] with pointer in struct cached_reg and struct reg_info, and allocate memory dynamically, or use std::vector if it helps.
I’ve just noticed I forgot to add a changelog for all of my patches. Apologies - I’ll add them for any V2 versions (or happy to repost all of them again with changelogs if required). On 12/12/2016 18:10, "Yao Qi" <qiyaoltc@gmail.com> wrote: >On 16-12-05 12:26:24, Alan Hayward wrote: >> This is part of a series adding AARCH64 SVE support to gdb and >>gdbserver. >> >> In SVE the maximum size of a variable-length vector register is 256 >>bytes, >> four >> times the current maximum size currently supported in gdb. This patch >> increases >> the max register size and max gdbserver buffer size accordingly. > >Joel expressed the willingness that we should make MAX_REGISTER_SIZE >gdbarch specific last time when it was changed from 32 to 64. >https://sourceware.org/ml/gdb-patches/2010-09/msg00245.html >I think we should make MAX_REGISTER_SIZE gdbarch specific, or stop >using it at all. I’m happy to do this if that’s what people want. I avoided doing it because I didn’t want to subtly break something and it’s going to be quite a large change - I might submit it a set of patches by itself. > >> >> Alternatively, I could add a target variable using gdbarch.c, however >> there are >> 80+ static arrays within the code using the value, which would all need >> replacing with mallocs/frees. > >We can use alloca to allocate memory on stack, and we can get the >size of a register if gdbarch and regnum is available. For example, >we can replace MAX_REGISTER_SIZE with register_size in >xtensa_pseudo_register_read this way, > > gdb_byte *buf = (gdb_byte *) alloca (register_size (gdbarch, >regnum)); > >This can be used many places to replace MAX_REGISTER_SIZE. Hopefully, >this may get rid of the use of MAX_REGISTER_SIZE except the uses in >python/py-unwind.c and remote.c. We may replace array >data[MAX_REGISTER_SIZE] >with pointer in struct cached_reg and struct reg_info, and allocate memory >dynamically, or use std::vector if it helps. > I’ll look into using this. Thanks for the review. Alan.
On 16-12-13 10:05:49, Alan Hayward wrote: > I’ve just noticed I forgot to add a changelog for all of my patches. > Apologies - I’ll add them for any V2 versions (or happy to repost all of > them again with changelogs if required). ChangeLog is not needed for V1. > > On 12/12/2016 18:10, "Yao Qi" <qiyaoltc@gmail.com> wrote: > > >On 16-12-05 12:26:24, Alan Hayward wrote: > >> This is part of a series adding AARCH64 SVE support to gdb and > >>gdbserver. > >> > >> In SVE the maximum size of a variable-length vector register is 256 > >>bytes, > >> four > >> times the current maximum size currently supported in gdb. This patch > >> increases > >> the max register size and max gdbserver buffer size accordingly. > > > >Joel expressed the willingness that we should make MAX_REGISTER_SIZE > >gdbarch specific last time when it was changed from 32 to 64. > >https://sourceware.org/ml/gdb-patches/2010-09/msg00245.html > >I think we should make MAX_REGISTER_SIZE gdbarch specific, or stop > >using it at all. > > I’m happy to do this if that’s what people want. I avoided doing it > because I > didn’t want to subtly break something and it’s going to be quite a large > change - > I might submit it a set of patches by itself. > You can start from changing amd64-tdep.c and frame.c, which are interesting to most of people here. It shouldn't take long to finish the patch, and post it to get feedback quickly. If people agree/like the change, then you can move on changing the rest in the same way.
> > >Joel expressed the willingness that we should make MAX_REGISTER_SIZE > > >gdbarch specific last time when it was changed from 32 to 64. > > >https://sourceware.org/ml/gdb-patches/2010-09/msg00245.html > > >I think we should make MAX_REGISTER_SIZE gdbarch specific, or stop > > >using it at all. > > > > I’m happy to do this if that’s what people want. I avoided doing it > > because I > > didn’t want to subtly break something and it’s going to be quite a large > > change - > > I might submit it a set of patches by itself. > > > > You can start from changing amd64-tdep.c and frame.c, which are > interesting to most of people here. It shouldn't take long to finish > the patch, and post it to get feedback quickly. If people agree/like > the change, then you can move on changing the rest in the same way. I agree. Let's separate this patch from the infrastructure rework. In particular, I don't remember at the time if I was considering the impact of making turning the max register size into a dynamic value - I am thinking a lot of code might be using it in expressions that assume it is constant (array of MAX_REGISTER_SIZE bytes). Do people agree that this is an idea worth pursuing? At the moment, I'm not sure myself...
diff --git a/gdb/defs.h b/gdb/defs.h index 3d21f62f52cc3a59d5effb62dcb78014acdfc092..a5ad024359f84391be0e79f143674439f d5c7f6f 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -622,7 +622,7 @@ enum symbol_needs_kind /* * Maximum size of a register. Something small, but large enough for all known ISAs. If it turns out to be too small, make it bigger. */ -enum { MAX_REGISTER_SIZE = 64 }; +enum { MAX_REGISTER_SIZE = 256 }; /* In findvar.c. */ diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index f56c0f5eca5a4a4bcba7789dde1dc41aa329fdfa..11cb3080dd7bc77231193d770df7b1c26 aa6751d 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -140,7 +140,7 @@ extern int in_queued_stop_replies (ptid_t ptid); /* Buffer sizes for transferring memory, registers, etc. Set to a constant value to accomodate multiple register formats. This value must be at least as large as the largest register set supported by gdbserver. */ -#define PBUFSIZ 16384 +#define PBUFSIZ 19200