> -----Original Message----- > From: Richard Henderson <richard.hender...@linaro.org> > Sent: Wednesday, August 26, 2020 9:07 AM > To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@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