Re: [PATCH,v2] SVE/FPSIMD fixup for big endian

[Luis Machado <luis.machado@linaro.org>]

On 12/8/20 1:10 PM, Simon Marchi wrote:
> On 2020-12-04 9:22 a.m., Luis Machado via Gdb-patches wrote:
>> With the AArch64-specific parts approved, do the global maintainers have any comments on the trad-frame changes, or does it look good?
> 
> The changes in trad-frame look good to me.  If you feel like it, you could
> change the bytes/size parameters in the functions' prototypes to use
> gdb::array_view.

I agree. The current interface is being abused a little. I'll push this 
fix first, then I'll do it as a separate patch. Might be useful for 
other targets as well, like ones that have larger vector types.

> 
> While reading your patch, I spotted some cleanups I'd like to see happening
> to make the code cleaner/safer (ideas for future patches).  The first one
> would be to make the fields private and add getters that assert that the kind
> of value you are getting is indeed the one stored, a bit like I did for the
> dynamic_prop structure.  That could catch some problems where we set value as
> bytes but fetch it as a register number.
> 
> The second one is: instead of overloading the realreg field to hold the kind
> of value that is stored in the object, and overloading the "addr" field for
> both LONGEST values and addresses, it would be much clearer to have a kind
> field + a union:
> 
> struct trad_frame_saved_reg
> {
>    trad_frame_saved_reg_kind kind;
> 
>    union
>      {
>        CORE_ADDR addr;
>        LONGEST value;
>        int regnum;
>        gdb_byte *data;
>      };
> };
> 
> It would take less space, too.
Thanks. That's good feedback. I'll take notes and will work on something 
to improve these structures/mechanisms.