Re: [PATCH 1/4] Hexagon (target/hexagon) Remove gen_log_reg_write

[Brian Cain <brian.cain@oss.qualcomm.com>]

[-- Attachment #1: Type: text/plain, Size: 13070 bytes --]

On Fri, Nov 14, 2025 at 5:00 PM Taylor Simpson <ltaylorsimpson@gmail.com>
wrote:

> The gen_log_reg_write function is a memnant of the original Hexagon
> target design.  With the addition of gen_analyze_funcs.py and the
> ability to short-circuit a packet commit, this function can be
> removed.
>
> Note that the implementation of gen_log_reg_write contains a check
> of the register mutability mask.  This is only needed for control
> registers, so we move it to gen_write_ctrl_reg.
>
> We do need the gen_log_reg_write_pair function, but the name is
> now misleading, so we change the name go gen_write_reg_pair.
>
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
>

Reviewed-by: Brian Cain <brian.cain@oss.qualcomm.com>


>  target/hexagon/gen_tcg.h                    |  7 +--
>  target/hexagon/genptr.h                     |  1 -
>  target/hexagon/genptr.c                     | 64 ++++++++-------------
>  target/hexagon/idef-parser/parser-helpers.c |  2 +-
>  target/hexagon/README                       | 10 ++--
>  target/hexagon/gen_tcg_funcs.py             |  1 -
>  target/hexagon/hex_common.py                | 14 ++---
>  7 files changed, 40 insertions(+), 59 deletions(-)
>
> diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
> index 8a3b801287..10123336b1 100644
> --- a/target/hexagon/gen_tcg.h
> +++ b/target/hexagon/gen_tcg.h
> @@ -509,10 +509,9 @@
>  /* sub-instruction version (no RxV, so handle it manually) */
>  #define fGEN_TCG_SS2_allocframe(SHORTCODE) \
>      do { \
> -        TCGv r29 = tcg_temp_new(); \
> +        TCGv r29 = get_result_gpr(ctx, HEX_REG_SP); \
>          tcg_gen_mov_tl(r29, hex_gpr[HEX_REG_SP]); \
>          gen_allocframe(ctx, r29, uiV); \
> -        gen_log_reg_write(ctx, HEX_REG_SP, r29); \
>      } while (0)
>
>  /*
> @@ -528,7 +527,7 @@
>      do { \
>          TCGv_i64 r31_30 = tcg_temp_new_i64(); \
>          gen_deallocframe(ctx, r31_30, hex_gpr[HEX_REG_FP]); \
> -        gen_log_reg_write_pair(ctx, HEX_REG_FP, r31_30); \
> +        gen_write_reg_pair(ctx, HEX_REG_FP, r31_30); \
>      } while (0)
>
>  /*
> @@ -546,7 +545,7 @@
>      do { \
>          TCGv_i64 RddV = get_result_gpr_pair(ctx, HEX_REG_FP); \
>          gen_return(ctx, RddV, hex_gpr[HEX_REG_FP]); \
> -        gen_log_reg_write_pair(ctx, HEX_REG_FP, RddV); \
> +        gen_write_reg_pair(ctx, HEX_REG_FP, RddV); \
>      } while (0)
>
>  /*
> diff --git a/target/hexagon/genptr.h b/target/hexagon/genptr.h
> index a4b43c2910..d932255042 100644
> --- a/target/hexagon/genptr.h
> +++ b/target/hexagon/genptr.h
> @@ -37,7 +37,6 @@ TCGv gen_read_reg(TCGv result, int num);
>  TCGv gen_read_preg(TCGv pred, uint8_t num);
>  TCGv get_result_gpr(DisasContext *ctx, int rnum);
>  TCGv get_result_pred(DisasContext *ctx, int pnum);
> -void gen_log_reg_write(DisasContext *ctx, int rnum, TCGv val);
>  void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val);
>  void gen_set_usr_field(DisasContext *ctx, int field, TCGv val);
>  void gen_set_usr_fieldi(DisasContext *ctx, int field, int x);
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index cecaece4ae..e58021ed6c 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -94,25 +94,17 @@ static TCGv_i64 get_result_gpr_pair(DisasContext *ctx,
> int rnum)
>      return result;
>  }
>
> -void gen_log_reg_write(DisasContext *ctx, int rnum, TCGv val)
> -{
> -    const target_ulong reg_mask = reg_immut_masks[rnum];
> -
> -    gen_masked_reg_write(val, hex_gpr[rnum], reg_mask);
> -    tcg_gen_mov_tl(get_result_gpr(ctx, rnum), val);
> -}
> -
> -static void gen_log_reg_write_pair(DisasContext *ctx, int rnum, TCGv_i64
> val)
> +static void gen_write_reg_pair(DisasContext *ctx, int rnum, TCGv_i64 val)
>  {
>      TCGv val32 = tcg_temp_new();
>
>      /* Low word */
>      tcg_gen_extrl_i64_i32(val32, val);
> -    gen_log_reg_write(ctx, rnum, val32);
> +    tcg_gen_mov_tl(get_result_gpr(ctx, rnum), val32);
>
>      /* High word */
>      tcg_gen_extrh_i64_i32(val32, val);
> -    gen_log_reg_write(ctx, rnum + 1, val32);
> +    tcg_gen_mov_tl(get_result_gpr(ctx, rnum + 1), val32);
>  }
>
>  TCGv get_result_pred(DisasContext *ctx, int pnum)
> @@ -240,7 +232,9 @@ static inline void gen_write_ctrl_reg(DisasContext
> *ctx, int reg_num,
>      if (reg_num == HEX_REG_P3_0_ALIASED) {
>          gen_write_p3_0(ctx, val);
>      } else {
> -        gen_log_reg_write(ctx, reg_num, val);
> +        const target_ulong reg_mask = reg_immut_masks[reg_num];
> +        gen_masked_reg_write(val, hex_gpr[reg_num], reg_mask);
> +        tcg_gen_mov_tl(get_result_gpr(ctx, reg_num), val);
>          if (reg_num == HEX_REG_QEMU_PKT_CNT) {
>              ctx->num_packets = 0;
>          }
> @@ -256,23 +250,15 @@ static inline void gen_write_ctrl_reg(DisasContext
> *ctx, int reg_num,
>  static inline void gen_write_ctrl_reg_pair(DisasContext *ctx, int reg_num,
>                                             TCGv_i64 val)
>  {
> -    if (reg_num == HEX_REG_P3_0_ALIASED) {
> -        TCGv result = get_result_gpr(ctx, reg_num + 1);
> -        TCGv val32 = tcg_temp_new();
> -        tcg_gen_extrl_i64_i32(val32, val);
> -        gen_write_p3_0(ctx, val32);
> -        tcg_gen_extrh_i64_i32(val32, val);
> -        tcg_gen_mov_tl(result, val32);
> -    } else {
> -        gen_log_reg_write_pair(ctx, reg_num, val);
> -        if (reg_num == HEX_REG_QEMU_PKT_CNT) {
> -            ctx->num_packets = 0;
> -            ctx->num_insns = 0;
> -        }
> -        if (reg_num == HEX_REG_QEMU_HVX_CNT) {
> -            ctx->num_hvx_insns = 0;
> -        }
> -    }
> +    TCGv val32 = tcg_temp_new();
> +
> +    /* Low word */
> +    tcg_gen_extrl_i64_i32(val32, val);
> +    gen_write_ctrl_reg(ctx, reg_num, val32);
> +
> +    /* High word */
> +    tcg_gen_extrh_i64_i32(val32, val);
> +    gen_write_ctrl_reg(ctx, reg_num + 1, val32);
>  }
>
>  TCGv gen_get_byte(TCGv result, int N, TCGv src, bool sign)
> @@ -541,8 +527,8 @@ static inline void gen_loop0r(DisasContext *ctx, TCGv
> RsV, int riV)
>  {
>      fIMMEXT(riV);
>      fPCALIGN(riV);
> -    gen_log_reg_write(ctx, HEX_REG_LC0, RsV);
> -    gen_log_reg_write(ctx, HEX_REG_SA0, tcg_constant_tl(ctx->pkt->pc +
> riV));
> +    tcg_gen_mov_tl(get_result_gpr(ctx, HEX_REG_LC0), RsV);
> +    tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA0), ctx->pkt->pc + riV);
>      gen_set_usr_fieldi(ctx, USR_LPCFG, 0);
>  }
>
> @@ -555,8 +541,8 @@ static inline void gen_loop1r(DisasContext *ctx, TCGv
> RsV, int riV)
>  {
>      fIMMEXT(riV);
>      fPCALIGN(riV);
> -    gen_log_reg_write(ctx, HEX_REG_LC1, RsV);
> -    gen_log_reg_write(ctx, HEX_REG_SA1, tcg_constant_tl(ctx->pkt->pc +
> riV));
> +    tcg_gen_mov_tl(get_result_gpr(ctx, HEX_REG_LC1), RsV);
> +    tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA1), ctx->pkt->pc + riV);
>  }
>
>  static void gen_loop1i(DisasContext *ctx, int count, int riV)
> @@ -568,8 +554,8 @@ static void gen_ploopNsr(DisasContext *ctx, int N,
> TCGv RsV, int riV)
>  {
>      fIMMEXT(riV);
>      fPCALIGN(riV);
> -    gen_log_reg_write(ctx, HEX_REG_LC0, RsV);
> -    gen_log_reg_write(ctx, HEX_REG_SA0, tcg_constant_tl(ctx->pkt->pc +
> riV));
> +    tcg_gen_mov_tl(get_result_gpr(ctx, HEX_REG_LC0), RsV);
> +    tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA0), ctx->pkt->pc + riV);
>      gen_set_usr_fieldi(ctx, USR_LPCFG, N);
>      gen_log_pred_write(ctx, 3, tcg_constant_tl(0));
>  }
> @@ -773,25 +759,23 @@ static void gen_framecheck(TCGv EA, int framesize)
>
>  static void gen_allocframe(DisasContext *ctx, TCGv r29, int framesize)
>  {
> -    TCGv r30 = tcg_temp_new();
> +    TCGv r30 = get_result_gpr(ctx, HEX_REG_FP);
>      TCGv_i64 frame;
>      tcg_gen_addi_tl(r30, r29, -8);
>      frame = gen_frame_scramble();
>      gen_store8(tcg_env, r30, frame, ctx->insn->slot);
> -    gen_log_reg_write(ctx, HEX_REG_FP, r30);
>      gen_framecheck(r30, framesize);
>      tcg_gen_subi_tl(r29, r30, framesize);
>  }
>
>  static void gen_deallocframe(DisasContext *ctx, TCGv_i64 r31_30, TCGv r30)
>  {
> -    TCGv r29 = tcg_temp_new();
> +    TCGv r29 = get_result_gpr(ctx, HEX_REG_SP);
>      TCGv_i64 frame = tcg_temp_new_i64();
>      gen_load_frame(ctx, frame, r30);
>      gen_frame_unscramble(frame);
>      tcg_gen_mov_i64(r31_30, frame);
>      tcg_gen_addi_tl(r29, r30, 8);
> -    gen_log_reg_write(ctx, HEX_REG_SP, r29);
>  }
>  #endif
>
> @@ -833,7 +817,7 @@ static void gen_cond_return_subinsn(DisasContext *ctx,
> TCGCond cond, TCGv pred)
>  {
>      TCGv_i64 RddV = get_result_gpr_pair(ctx, HEX_REG_FP);
>      gen_cond_return(ctx, RddV, hex_gpr[HEX_REG_FP], pred, cond);
> -    gen_log_reg_write_pair(ctx, HEX_REG_FP, RddV);
> +    gen_write_reg_pair(ctx, HEX_REG_FP, RddV);
>  }
>
>  static void gen_endloop0(DisasContext *ctx)
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> index 1dc52b4e02..f5802ceadb 100644
> --- a/target/hexagon/idef-parser/parser-helpers.c
> +++ b/target/hexagon/idef-parser/parser-helpers.c
> @@ -1315,7 +1315,7 @@ void gen_write_reg(Context *c, YYLTYPE *locp,
> HexValue *reg, HexValue *value)
>      value_m = rvalue_materialize(c, locp, &value_m);
>      OUT(c,
>          locp,
> -        "gen_log_reg_write(ctx, ", &reg->reg.id, ", ",
> +        "tcg_gen_mov_tl(get_result_gpr(ctx, ", &reg->reg.id, "), ",
>          &value_m, ");\n");
>  }
>
> diff --git a/target/hexagon/README b/target/hexagon/README
> index ca617e3364..1938c91af8 100644
> --- a/target/hexagon/README
> +++ b/target/hexagon/README
> @@ -80,12 +80,14 @@ tcg_funcs_generated.c.inc
>                      Insn *insn,
>                      Packet *pkt)
>      {
> -        TCGv RdV = tcg_temp_new();
> +        Insn *insn G_GNUC_UNUSED = ctx->insn;
>          const int RdN = insn->regno[0];
> -        TCGv RsV = hex_gpr[insn->regno[1]];
> -        TCGv RtV = hex_gpr[insn->regno[2]];
> +        TCGv RdV = get_result_gpr(ctx, RdN);
> +        const int RsN = insn->regno[1];
> +        TCGv RsV = hex_gpr[RsN];
> +        const int RtN = insn->regno[2];
> +        TCGv RtV = hex_gpr[RtN];
>          gen_helper_A2_add(RdV, tcg_env, RsV, RtV);
> -        gen_log_reg_write(ctx, RdN, RdV);
>      }
>
>  helper_funcs_generated.c.inc
> diff --git a/target/hexagon/gen_tcg_funcs.py
> b/target/hexagon/gen_tcg_funcs.py
> index c2ba91ddc0..bd241afde1 100755
> --- a/target/hexagon/gen_tcg_funcs.py
> +++ b/target/hexagon/gen_tcg_funcs.py
> @@ -35,7 +35,6 @@
>  ##        TCGv RsV = hex_gpr[insn->regno[1]];
>  ##        TCGv RtV = hex_gpr[insn->regno[2]];
>  ##        <GEN>
> -##        gen_log_reg_write(ctx, RdN, RdV);
>  ##    }
>  ##
>  ##       where <GEN> depends on hex_common.skip_qemu_helper(tag)
> diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
> index 6803908718..093def9386 100755
> --- a/target/hexagon/hex_common.py
> +++ b/target/hexagon/hex_common.py
> @@ -452,9 +452,8 @@ def decl_tcg(self, f, tag, regno):
>              TCGv {self.reg_tcg()} = get_result_gpr(ctx, {self.reg_num});
>          """))
>      def log_write(self, f, tag):
> -        f.write(code_fmt(f"""\
> -            gen_log_reg_write(ctx, {self.reg_num}, {self.reg_tcg()});
> -        """))
> +        ## No write needed
> +        return
>      def analyze_write(self, f, tag, regno):
>          predicated = "true" if is_predicated(tag) else "false"
>          f.write(code_fmt(f"""\
> @@ -496,9 +495,8 @@ def decl_tcg(self, f, tag, regno):
>                  tcg_gen_mov_tl({self.reg_tcg()}, hex_gpr[{self.reg_num}]);
>              """))
>      def log_write(self, f, tag):
> -        f.write(code_fmt(f"""\
> -            gen_log_reg_write(ctx, {self.reg_num}, {self.reg_tcg()});
> -        """))
> +        ## No write needed
> +        return
>      def analyze_read(self, f, regno):
>          f.write(code_fmt(f"""\
>              ctx_log_reg_read(ctx, {self.reg_num});
> @@ -630,7 +628,7 @@ def decl_tcg(self, f, tag, regno):
>          """))
>      def log_write(self, f, tag):
>          f.write(code_fmt(f"""\
> -            gen_log_reg_write_pair(ctx, {self.reg_num}, {self.reg_tcg()});
> +            gen_write_reg_pair(ctx, {self.reg_num}, {self.reg_tcg()});
>          """))
>      def analyze_write(self, f, tag, regno):
>          predicated = "true" if is_predicated(tag) else "false"
> @@ -664,7 +662,7 @@ def decl_tcg(self, f, tag, regno):
>          """))
>      def log_write(self, f, tag):
>          f.write(code_fmt(f"""\
> -            gen_log_reg_write_pair(ctx, {self.reg_num}, {self.reg_tcg()});
> +            gen_write_reg_pair(ctx, {self.reg_num}, {self.reg_tcg()});
>          """))
>      def analyze_read(self, f, regno):
>          f.write(code_fmt(f"""\
> --
> 2.43.0
>
>

[-- Attachment #2: Type: text/html, Size: 15622 bytes --]