RE: [RFC PATCH v3 14/34] Hexagon (target/hexagon) instruction/packet decode

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, August 26, 2020 9:07 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 14/34] Hexagon (target/hexagon)
> instruction/packet decode
>
> On 8/18/20 8:50 AM, Taylor Simpson wrote:
> > +#define DECODE_NEW_TABLE(TAG, SIZE, WHATNOT) \
> > +    static struct _dectree_table_struct dectree_table_##TAG;
>
> All of these little structures should be const.
>
> Which probably requires these links to be const, and a few uses within the
> functions that use them.
>
> That should move 78K worth of tables into .data.rel.ro, where they can't be
> modified after relocation.

OK

> I can't help but thinking that all of this string manipulation is not the most
> ideal way to implement a decoder.  Surely all this can be pre-processed into
> code, or flags, or an enumeration or something.

I'll have to think about this one.

>
> Oh, and g_assert_not_reached() will never return.  You don't have to keep
> putting return statements after it.  Please remove all of those, everywhere.

OK

> I'm going to ignore the rest of the decoder, as it's somewhat bewildering.
> Even in the final patches-applied form.  It could really use some more
> commentary.

OK