[PATCH 1/8] AARCH64 SVE: Increse max register sizes

[PATCH 1/8] AARCH64 SVE: Increse max register sizes

[Alan Hayward]

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.



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

 /* Definition for an unknown syscall, used basically in error-cases.  */
 #define UNKNOWN_SYSCALL (-1)

Re: [PATCH 1/8] AARCH64 SVE: Increse max register sizes

[Yao Qi]

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.

-- 
Yao (齐尧)

Re: [PATCH 1/8] AARCH64 SVE: Increse max register sizes

[Alan Hayward]

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.

Re: [PATCH 1/8] AARCH64 SVE: Increse max register sizes

[Yao Qi]

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.

-- 
Yao (齐尧)

Re: [PATCH 1/8] AARCH64 SVE: Increse max register sizes

[Joel Brobecker]

> > >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...

-- 
Joel