[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [RFC PATCH v3 27/34] Hexagon (target/hexagon) instruction classes
From: |
Taylor Simpson |
Subject: |
RE: [RFC PATCH v3 27/34] Hexagon (target/hexagon) instruction classes |
Date: |
Sun, 30 Aug 2020 20:04:31 +0000 |
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, August 28, 2020 7:37 PM
> 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 27/34] Hexagon (target/hexagon) instruction
> classes
>
> On 8/18/20 8:50 AM, Taylor Simpson wrote:
> > +} iclass_t;
> ...
> > +extern const char *find_iclass_slots(opcode_t opcode, int itype);
> ...
> > +typedef struct {
> > + const char * const slots;
> > +} iclass_info_t;
>
> I'll note that you aren't following our CODING_STYLE for types. Which of
> these
> need to match imported/ and which can you fix.
So, this should be CamelCase? I should be able to fix all of them.
>
> > +typedef enum {
> > +
> > +#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)
> ICLASS_FROM_TYPE(TYPE),
> > +#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS) /* nothing */
> > +#include "imported/iclass.def"
> > +#undef DEF_PP_ICLASS32
> > +#undef DEF_EE_ICLASS32
> > +
> > +#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)
> ICLASS_FROM_TYPE(TYPE),
> > +#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS) /* nothing */
> > +#include "imported/iclass.def"
> > +#undef DEF_PP_ICLASS32
> > +#undef DEF_EE_ICLASS32
>
> Any particular reason why you're defining one as nothing, and doing this
> dance
> twice?
Will investigate.
> > +const char *find_iclass_slots(opcode_t opcode, int itype)
> > +{
> > + /* There are some exceptions to what the iclass dictates */
> > + if (GET_ATTRIB(opcode, A_ICOP)) {
> > + return "2";
>
> Why are you returning a string and not a simple bitmask?
Will fix
> > + [ICLASS_FROM_TYPE(TYPE)] = { .slots = #SLOTS },
>
> This could be converted to the bitmask with
>
> enum {
> SLOTS_0 = (1 << 0),
> SLOTS_1 = (1 << 1),
> SLOTS_23 = (1 << 2) | (1 << 3),
> ...
> };
>
> static const uint8_t iclass_info[] = {
>
> #define DEF_ICLASS(TYPE, SLOTS) \
> [ICLASS_##TYPE] = SLOTS_##SLOTS
> #define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS) \
> DEF_ICLASS(TYPE, SLOTS)
> #define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS) \
> DEF_ICLASS(TYPE, SLOTS)
>
> At which point the ultimate consumer,
>
> > for (i = 0, slot = 3; i < pkt->num_insns; i++) {
> > valid_slot_str = get_valid_slot_str(pkt, i);
> >
> > while (strchr(valid_slot_str, '0' + slot) == NULL) {
> > slot--;
> > }
> > pkt->insn[i].slot = slot;
>
> becomes a simple integer mask check.
Will fix
[RFC PATCH v3 21/34] Hexagon (target/hexagon) generator phase 2 - generate header files, Taylor Simpson, 2020/08/18
[RFC PATCH v3 22/34] Hexagon (target/hexagon) generator phase 3 - C preprocessor for decode tree, Taylor Simpson, 2020/08/18
[RFC PATCH v3 32/34] Hexagon (linux-user/hexagon) Linux user emulation, Taylor Simpson, 2020/08/18
[RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2020/08/18