RE: [RFC PATCH v3 24/34] Hexagon (target/hexagon) opcode data structures

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, August 26, 2020 9:26 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 24/34] Hexagon (target/hexagon) opcode data
> structures
>
> > +extern const char *opcode_names[];
> > +
> > +extern const char *opcode_reginfo[];
> > +extern const char *opcode_rregs[];
> > +extern const char *opcode_wregs[];
>
> const char * const

OK

> > +extern opcode_encoding_t opcode_encodings[XX_LAST_OPCODE];
>
> const.

OK

> > +extern size4u_t
> > +    opcode_attribs[XX_LAST_OPCODE][(A_ZZ_LASTATTRIB /
> ATTRIB_WIDTH) + 1];
>
> const.

OK

> And using qemu/bitops.h if possible, as discussed earlier vs attribs.h.

Do you mean replace the GET_ATTRIB macro with test_bit from qemu/bitops.h?

> Just initialize opcode_short_semantics with shortcode_generated.h in the
> first
> place.  Then you don't need to create a table of NULL and overwrite at
> startup.
>
> And you can also make the table constant.

OK

> > +    if (p == NULL) {
> > +        g_assert_not_reached();
> > +        return 0;
> > +    }
>
> I prefer assert(p != NULL) to if (test) { abort(); }, where possible.  E.g.
> this later one is fine:

OK