Re: [PATCH 2/3] Allocate data in cached_reg_t

[Alan Hayward <Alan.Hayward@arm.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2085 bytes --]


> On 10 Jan 2017, at 17:42, Luis Machado <lgustavo@codesourcery.com> wrote:
> 
> On 01/10/2017 06:59 AM, Yao Qi wrote:
>> On 17-01-09 14:11:13, Luis Machado wrote:
>>>> @@ -6306,7 +6306,7 @@ remote_console_output (char *msg)
>>>> typedef struct cached_reg
>>>> {
>>>>  int num;
>>>> -  gdb_byte data[MAX_REGISTER_SIZE];
>>>> +  gdb_byte *data;
>>> 
>>> Would it make sense to go C++ and use a data structure that can take
>>> care of variable sizes? Just thinking if that would be easier than
>>> handling allocation/deallocation of the data.
>>> 
>> 
>> Do you suggest std::vector?  We still need to allocate/deallocate it
>> if we change "gdb_byte *data" to "std::vector<gdb_byte> *data", or we
>> have to convert cached_reg to class, and do RAII.
>> 
> 
> Something like std::vector, yes. But it is true we will still need to allocate the vector itself.
> 
> I was pondering about the benefits of not being limited to a specific register size. Then we wouldn't need to worry about adjusting things as we have to do now.
> 
> As i see it, data[register_size(register_size (gdbarch, num)] wouldn't be much different than data[MAX_REGISTER_SIZE]. The only thing is that we're setting the max register size dynamically.
> 
> We may or may not need this flexibility right now, but who knows what weird architectures we may have in the future.

I avoided using data[register_size(gdbarch, num)] as dynamically sized arrays are a gcc extension - it’s not something we can expect to work on other compilers.
Plus there is a danger of it using up all the stack. Aarch64 SVE will have a max register size of 256, potentially this could grow further in the future.
This is why I used alloca in patches 1/3 and 2/3.

I know that gcc avoid using std::vector because it’s fairly slow. I think gcc also have newer c++ like replacements for VEC, but am not sure what they have that’s suitable. However, I’d suggest that’s something that should be done as part of the work to move gdb to c++, and not this patch.

Alan.\x16º&Öéj×!zÊÞ¶êç×~µÛ¹b²Ö«r\x18\x1dn–­r\x17¬