RE: [PATCH 1/9] Hexagon (target/hexagon) Clean up handling of modifier registers

[Brian Cain]

> -----Original Message-----
> From: Taylor Simpson <ltaylorsimpson@gmail.com>
> Sent: Monday, December 4, 2023 7:53 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 1/9] Hexagon (target/hexagon) Clean up handling of modifier
> registers
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
> 
> Currently, the register number (MuN) for modifier registers is the
> modifier register number rather than the index into hex_gpr.  This
> patch changes MuN to the hex_gpr index, which is consistent with
> the handling of control registers.
> 
> Note that HELPER(fcircadd) needs the CS register corresponding to the
> modifier register specified in the instruction.  We create a TCGv
> variable "CS" to hold the value to pass to the helper.
> 
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
>  target/hexagon/gen_tcg.h                    |  9 ++++-----
>  target/hexagon/macros.h                     |  3 +--
>  target/hexagon/idef-parser/parser-helpers.c |  8 +++-----
>  target/hexagon/gen_tcg_funcs.py             | 13 +++++++++----
>  4 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
> index d992059fce..1c4391b415 100644
> --- a/target/hexagon/gen_tcg.h
> +++ b/target/hexagon/gen_tcg.h
> @@ -68,15 +68,14 @@
>      do { \
>          TCGv tcgv_siV = tcg_constant_tl(siV); \
>          tcg_gen_mov_tl(EA, RxV); \
> -        gen_helper_fcircadd(RxV, RxV, tcgv_siV, MuV, \
> -                            hex_gpr[HEX_REG_CS0 + MuN]); \
> +        gen_helper_fcircadd(RxV, RxV, tcgv_siV, MuV, CS); \
>      } while (0)
>  #define GET_EA_pcr(SHIFT) \
>      do { \
>          TCGv ireg = tcg_temp_new(); \
>          tcg_gen_mov_tl(EA, RxV); \
>          gen_read_ireg(ireg, MuV, (SHIFT)); \
> -        gen_helper_fcircadd(RxV, RxV, ireg, MuV, hex_gpr[HEX_REG_CS0 +
> MuN]); \
> +        gen_helper_fcircadd(RxV, RxV, ireg, MuV, CS); \
>      } while (0)
> 
>  /* Instructions with multiple definitions */
> @@ -113,7 +112,7 @@
>          TCGv ireg = tcg_temp_new(); \
>          tcg_gen_mov_tl(EA, RxV); \
>          gen_read_ireg(ireg, MuV, SHIFT); \
> -        gen_helper_fcircadd(RxV, RxV, ireg, MuV, hex_gpr[HEX_REG_CS0 +
> MuN]); \
> +        gen_helper_fcircadd(RxV, RxV, ireg, MuV, CS); \
>          LOAD; \
>      } while (0)
> 
> @@ -427,7 +426,7 @@
>          TCGv BYTE G_GNUC_UNUSED = tcg_temp_new(); \
>          tcg_gen_mov_tl(EA, RxV); \
>          gen_read_ireg(ireg, MuV, SHIFT); \
> -        gen_helper_fcircadd(RxV, RxV, ireg, MuV, hex_gpr[HEX_REG_CS0 +
> MuN]); \
> +        gen_helper_fcircadd(RxV, RxV, ireg, MuV, CS); \
>          STORE; \
>      } while (0)
> 
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> index 9a51b5709b..939f22e76b 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -462,8 +462,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val,
> int shift)
>  #define fPM_CIRI(REG, IMM, MVAL) \
>      do { \
>          TCGv tcgv_siV = tcg_constant_tl(siV); \
> -        gen_helper_fcircadd(REG, REG, tcgv_siV, MuV, \
> -                            hex_gpr[HEX_REG_CS0 + MuN]); \
> +        gen_helper_fcircadd(REG, REG, tcgv_siV, MuV, CS); \
>      } while (0)
>  #else
>  #define fEA_IMM(IMM)        do { EA = (IMM); } while (0)
> diff --git a/target/hexagon/idef-parser/parser-helpers.c 
> b/target/hexagon/idef-
> parser/parser-helpers.c
> index 4af020933a..95f2b43076 100644
> --- a/target/hexagon/idef-parser/parser-helpers.c
> +++ b/target/hexagon/idef-parser/parser-helpers.c
> @@ -1541,10 +1541,8 @@ void gen_circ_op(Context *c,
>                   HexValue *increment,
>                   HexValue *modifier)
>  {
> -    HexValue cs = gen_tmp(c, locp, 32, UNSIGNED);
>      HexValue increment_m = *increment;
>      increment_m = rvalue_materialize(c, locp, &increment_m);
> -    OUT(c, locp, "gen_read_reg(", &cs, ", HEX_REG_CS0 + MuN);\n");
>      OUT(c,
>          locp,
>          "gen_helper_fcircadd(",
> @@ -1555,7 +1553,7 @@ void gen_circ_op(Context *c,
>          &increment_m,
>          ", ",
>          modifier);
> -    OUT(c, locp, ", ", &cs, ");\n");
> +    OUT(c, locp, ", CS);\n");
>  }
> 
>  HexValue gen_locnt_op(Context *c, YYLTYPE *locp, HexValue *src)
> @@ -2080,9 +2078,9 @@ void emit_arg(Context *c, YYLTYPE *locp, HexValue
> *arg)
>              char reg_id[5];
>              reg_compose(c, locp, &(arg->reg), reg_id);
>              EMIT_SIG(c, ", %s %s", type, reg_id);
> -            /* MuV register requires also MuN to provide its index */
> +            /* MuV register requires also CS for circular addressing*/
>              if (arg->reg.type == MODIFIER) {
> -                EMIT_SIG(c, ", int MuN");
> +                EMIT_SIG(c, ", TCGv CS");
>              }
>          }
>          break;
> diff --git a/target/hexagon/gen_tcg_funcs.py
> b/target/hexagon/gen_tcg_funcs.py
> index f5246cee6d..02d93bc5ce 100755
> --- a/target/hexagon/gen_tcg_funcs.py
> +++ b/target/hexagon/gen_tcg_funcs.py
> @@ -99,10 +99,15 @@ def genptr_decl(f, tag, regtype, regid, regno):
>              hex_common.bad_register(regtype, regid)
>      elif regtype == "M":
>          if regid == "u":
> -            f.write(f"    const int {regtype}{regid}N = " 
> f"insn->regno[{regno}];\n")
>              f.write(
> -                f"    TCGv {regtype}{regid}V = hex_gpr[{regtype}{regid}N + "
> -                "HEX_REG_M0];\n"
> +                f"    const int {regN} = insn->regno[{regno}] + 
> HEX_REG_M0;\n"
> +            )
> +            f.write(
> +                f"    TCGv {regtype}{regid}V = hex_gpr[{regN}];\n"
> +            )
> +            f.write(
> +                f"    TCGv CS G_GNUC_UNUSED = "
> +                f"hex_gpr[{regN} - HEX_REG_M0 + HEX_REG_CS0];\n"
>              )
>          else:
>              hex_common.bad_register(regtype, regid)
> @@ -528,7 +533,7 @@ def gen_tcg_func(f, tag, regs, imms):
>              ):
>                  declared.append(f"{regtype}{regid}V")
>                  if regtype == "M":
> -                    declared.append(f"{regtype}{regid}N")
> +                    declared.append("CS")
>              elif hex_common.is_new_val(regtype, regid, tag):
>                  declared.append(f"{regtype}{regid}N")
>              else:
> --
> 2.34.1

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