Re: [PATCH v5 12/14] Hexagon (target/hexagon) Remove gen_log_predicated_reg_write[_pair]

[Anton Johansson]

On 1/31/23 23:56, Taylor Simpson wrote:
>   static void gen_log_reg_write_pair(int rnum, TCGv_i64 val)
>   {
>       const target_ulong reg_mask_low = reg_immut_masks[rnum];
> @@ -167,6 +120,7 @@ static void gen_log_reg_write_pair(int rnum, TCGv_i64 val)
>       }
>   
>       tcg_temp_free(val32);
> +    tcg_temp_free_i64(val);
>   }
>   
>   void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val)
> @@ -306,12 +260,14 @@ 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);
> -        gen_log_reg_write(reg_num + 1, val32);
> +        tcg_gen_mov_tl(result, val32);
>           tcg_temp_free(val32);
> +        tcg_temp_free_i64(val);
>       } else {
>           gen_log_reg_write_pair(reg_num, val);
>           if (reg_num == HEX_REG_QEMU_PKT_CNT) {

Hiding the free of a TCGv argument scares me a bit, it's bound to cause 
annoying bugs down
the line, I feel.

Any reason we can't keep the frees in the same scope as allocation?

Otherwise, the patch looks good,

Reviewed-by: Anton Johansson <anjo@rev.ng>