RE: [RFC PATCH v3 11/34] Hexagon (target/hexagon) register fields

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, August 26, 2020 8:30 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com; laurent@vivier.eu; riku.voipio@iki.fi;
> aleksandar.m.mail@gmail.com; ale@rev.ng
> Subject: Re: [RFC PATCH v3 11/34] Hexagon (target/hexagon) register fields
>
> > +#define NUM_GEN_REGS 32
>
> What's this?  It doesn't appear to be field related.

Not needed, will remove it.

> > +extern reg_field_t reg_field_info[];
>
> const.

OK

> > +enum reg_fields_enum {
>
> Doesn't follow naming guidelines.  But you don't actually use the name at all,
> so better to just drop the name entirely?

OK

> > +/* USR fields */
> > +DEF_REG_FIELD(USR_OVF,
> > +    "ovf", 0, 1,
> > +    "Sticky Saturation Overflow - "
> > +    "Set when saturation occurs while executing instruction that specifies 
> > "
> > +    "optional saturation, remains set until explicitly cleared by a USR=Rs 
> > "
> > +    "instruction.")
>
> Is the description as a string really useful, or even used?
> A comment would seem to do just as well, not consume space in the final
> binary,
> and even then seems redundant with the actual architecture manual.

I thought they help make the code more readable.  You are right that they 
shouldn't take space in the binary.

I can either change so they don't go into the binary or remove them altogether 
- guess I'll remove them altogether.