RE: [PATCH v2 6/9] Hexagon (target/hexagon) Remove uses of op_regs_generated.h.inc

[Brian Cain]

> -----Original Message-----
> From: Taylor Simpson <ltaylorsimpson@gmail.com>
> Sent: Wednesday, March 6, 2024 9:23 PM
> To: qemu-devel@nongnu.org
> Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>;
> Marco Liebel (QUIC) <quic_mliebel@quicinc.com>;
> richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; anjo@rev.ng;
> ltaylorsimpson@gmail.com
> Subject: [PATCH v2 6/9] Hexagon (target/hexagon) Remove uses of
> op_regs_generated.h.inc
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---

Reviewed-by: Brian Cain <bcain@quicinc.com>

>  target/hexagon/opcodes.h                |  4 --
>  target/hexagon/decode.c                 | 57 +++----------------------
>  target/hexagon/mmvec/decode_ext_mmvec.c | 34 +++------------
>  target/hexagon/opcodes.c                | 28 ------------
>  4 files changed, 13 insertions(+), 110 deletions(-)
> 
> diff --git a/target/hexagon/opcodes.h b/target/hexagon/opcodes.h
> index fa7e321950..0ee11bd445 100644
> --- a/target/hexagon/opcodes.h
> +++ b/target/hexagon/opcodes.h
> @@ -40,10 +40,6 @@ typedef enum {
> 
>  extern const char * const opcode_names[];
> 
> -extern const char * const opcode_reginfo[];
> -extern const char * const opcode_rregs[];
> -extern const char * const opcode_wregs[];
> -
>  typedef struct {
>      const char * const encoding;
>      const EncClass enc_class;
> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
> index 84a3899556..23deba2426 100644
> --- a/target/hexagon/decode.c
> +++ b/target/hexagon/decode.c
> @@ -115,24 +115,13 @@ static void
>  decode_fill_newvalue_regno(Packet *packet)
>  {
>      int i, use_regidx, offset, def_idx, dst_idx;
> -    uint16_t def_opcode, use_opcode;
> -    char *dststr;
> 
>      for (i = 1; i < packet->num_insns; i++) {
>          if (GET_ATTRIB(packet->insn[i].opcode, A_DOTNEWVALUE) &&
>              !GET_ATTRIB(packet->insn[i].opcode, A_EXTENSION)) {
> -            use_opcode = packet->insn[i].opcode;
> -
> -            /* It's a store, so we're adjusting the Nt field */
> -            if (GET_ATTRIB(use_opcode, A_STORE)) {
> -                use_regidx = strchr(opcode_reginfo[use_opcode], 't') -
> -                    opcode_reginfo[use_opcode];
> -            } else {    /* It's a Jump, so we're adjusting the Ns field */
> -                use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
> -                    opcode_reginfo[use_opcode];
> -            }
> -            g_assert(packet->insn[i].new_read_idx != -1 &&
> -                     packet->insn[i].new_read_idx == use_regidx);
> +
> +            g_assert(packet->insn[i].new_read_idx != -1);
> +            use_regidx = packet->insn[i].new_read_idx;
> 
>              /*
>               * What's encoded at the N-field is the offset to who's producing
> @@ -153,39 +142,9 @@ decode_fill_newvalue_regno(Packet *packet)
>               */
>              g_assert(!((def_idx < 0) || (def_idx > (packet->num_insns - 
> 1))));
> 
> -            /*
> -             * packet->insn[def_idx] is the producer
> -             * Figure out which type of destination it produces
> -             * and the corresponding index in the reginfo
> -             */
> -            def_opcode = packet->insn[def_idx].opcode;
> -            dststr = strstr(opcode_wregs[def_opcode], "Rd");
> -            if (dststr) {
> -                dststr = strchr(opcode_reginfo[def_opcode], 'd');
> -            } else {
> -                dststr = strstr(opcode_wregs[def_opcode], "Rx");
> -                if (dststr) {
> -                    dststr = strchr(opcode_reginfo[def_opcode], 'x');
> -                } else {
> -                    dststr = strstr(opcode_wregs[def_opcode], "Re");
> -                    if (dststr) {
> -                        dststr = strchr(opcode_reginfo[def_opcode], 'e');
> -                    } else {
> -                        dststr = strstr(opcode_wregs[def_opcode], "Ry");
> -                        if (dststr) {
> -                            dststr = strchr(opcode_reginfo[def_opcode], 'y');
> -                        } else {
> -                            g_assert_not_reached();
> -                        }
> -                    }
> -                }
> -            }
> -            g_assert(dststr != NULL);
> -
>              /* Now patch up the consumer with the register number */
> -            dst_idx = dststr - opcode_reginfo[def_opcode];
> -            g_assert(packet->insn[def_idx].dest_idx != -1 &&
> -                     packet->insn[def_idx].dest_idx == dst_idx);
> +            g_assert(packet->insn[def_idx].dest_idx != -1);
> +            dst_idx = packet->insn[def_idx].dest_idx;
>              packet->insn[i].regno[use_regidx] =
>                  packet->insn[def_idx].regno[dst_idx];
>              /*
> @@ -366,11 +325,7 @@ static void decode_shuffle_for_execution(Packet
> *packet)
>          for (flag = false, i = 0; i < last_insn + 1; i++) {
>              int opcode = packet->insn[i].opcode;
> 
> -            g_assert(packet->insn[i].has_pred_dest ==
> -                     (strstr(opcode_wregs[opcode], "Pd4") ||
> -                      strstr(opcode_wregs[opcode], "Pe4")));
> -            if ((strstr(opcode_wregs[opcode], "Pd4") ||
> -                 strstr(opcode_wregs[opcode], "Pe4")) &&
> +            if (packet->insn[i].has_pred_dest &&
>                  GET_ATTRIB(opcode, A_STORE) == 0) {
>                  /* This should be a compare (not a store conditional) */
>                  if (flag) {
> diff --git a/target/hexagon/mmvec/decode_ext_mmvec.c
> b/target/hexagon/mmvec/decode_ext_mmvec.c
> index c1320406df..f850d0154d 100644
> --- a/target/hexagon/mmvec/decode_ext_mmvec.c
> +++ b/target/hexagon/mmvec/decode_ext_mmvec.c
> @@ -28,21 +28,15 @@ check_new_value(Packet *pkt)
>  {
>      /* .new value for a MMVector store */
>      int i, j;
> -    const char *reginfo;
> -    const char *destletters;
> -    const char *dststr = NULL;
>      uint16_t def_opcode;
> -    char letter;
> 
>      for (i = 1; i < pkt->num_insns; i++) {
>          uint16_t use_opcode = pkt->insn[i].opcode;
>          if (GET_ATTRIB(use_opcode, A_DOTNEWVALUE) &&
>              GET_ATTRIB(use_opcode, A_CVI) &&
>              GET_ATTRIB(use_opcode, A_STORE)) {
> -            int use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
> -                opcode_reginfo[use_opcode];
> -            g_assert(pkt->insn[i].new_read_idx != -1 &&
> -                     pkt->insn[i].new_read_idx == use_regidx);
> +            int use_regidx = pkt->insn[i].new_read_idx;
> +            g_assert(pkt->insn[i].new_read_idx != -1);
>              /*
>               * What's encoded at the N-field is the offset to who's producing
>               * the value.
> @@ -70,33 +64,19 @@ check_new_value(Packet *pkt)
> 
>              /* def_idx is the index of the producer */
>              def_opcode = pkt->insn[def_idx].opcode;
> -            reginfo = opcode_reginfo[def_opcode];
> -            destletters = "dexy";
> -            for (j = 0; (letter = destletters[j]) != 0; j++) {
> -                dststr = strchr(reginfo, letter);
> -                if (dststr != NULL) {
> -                    break;
> -                }
> -            }
> -            if ((dststr == NULL)  && GET_ATTRIB(def_opcode, A_CVI_GATHER)) {
> +            if ((pkt->insn[def_idx].dest_idx == -1)  &&
> +                GET_ATTRIB(def_opcode, A_CVI_GATHER)) {
>                  pkt->insn[i].regno[use_regidx] = def_oreg;
>                  pkt->insn[i].new_value_producer_slot = 
> pkt->insn[def_idx].slot;
>              } else {
> -                if (dststr == NULL) {
> +                if (pkt->insn[def_idx].dest_idx == -1) {
>                      /* still not there, we have a bad packet */
>                      g_assert_not_reached();
>                  }
> -                g_assert(pkt->insn[def_idx].dest_idx != -1 &&
> -                         pkt->insn[def_idx].dest_idx == dststr - reginfo);
> -                int def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> +                int def_regnum =
> +                    pkt->insn[def_idx].regno[pkt->insn[def_idx].dest_idx];
>                  /* Now patch up the consumer with the register number */
>                  pkt->insn[i].regno[use_regidx] = def_regnum ^ def_oreg;
> -                /* special case for (Vx,Vy) */
> -                dststr = strchr(reginfo, 'y');
> -                if (def_oreg && strchr(reginfo, 'x') && dststr) {
> -                    def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> -                    pkt->insn[i].regno[use_regidx] = def_regnum;
> -                }
>                  /*
>                   * We need to remember who produces this value to later
>                   * check if it was dynamically cancelled
> diff --git a/target/hexagon/opcodes.c b/target/hexagon/opcodes.c
> index 1f7f3def38..02ae9cf787 100644
> --- a/target/hexagon/opcodes.c
> +++ b/target/hexagon/opcodes.c
> @@ -36,34 +36,6 @@ const char * const opcode_names[] = {
>  #undef OPCODE
>  };
> 
> -const char * const opcode_reginfo[] = {
> -#define IMMINFO(TAG, SIGN, SIZE, SHAMT, SIGN2, SIZE2, SHAMT2)    /*
> nothing */
> -#define REGINFO(TAG, REGINFO, RREGS, WREGS) REGINFO,
> -#include "op_regs_generated.h.inc"
> -    NULL
> -#undef REGINFO
> -#undef IMMINFO
> -};
> -
> -
> -const char * const opcode_rregs[] = {
> -#define IMMINFO(TAG, SIGN, SIZE, SHAMT, SIGN2, SIZE2, SHAMT2)    /*
> nothing */
> -#define REGINFO(TAG, REGINFO, RREGS, WREGS) RREGS,
> -#include "op_regs_generated.h.inc"
> -    NULL
> -#undef REGINFO
> -#undef IMMINFO
> -};
> -
> -
> -const char * const opcode_wregs[] = {
> -#define IMMINFO(TAG, SIGN, SIZE, SHAMT, SIGN2, SIZE2, SHAMT2)    /*
> nothing */
> -#define REGINFO(TAG, REGINFO, RREGS, WREGS) WREGS,
> -#include "op_regs_generated.h.inc"
> -    NULL
> -#undef REGINFO
> -#undef IMMINFO
> -};
> 
>  const char * const opcode_short_semantics[] = {
>  #define DEF_SHORTCODE(TAG, SHORTCODE)              [TAG] = #SHORTCODE,
> --
> 2.34.1