Re: [PATCH 1/3] target/mips: Use tcg_constant_tl() instead of tcg_gen_movi_tl()

[Richard Henderson <richard.henderson@linaro.org>]

On 10/4/24 13:26, Philippe Mathieu-Daudé wrote:
> @@ -11428,17 +11415,12 @@ static void gen_compute_compact_branch(DisasContext *ctx, uint32_t opc,
>   void gen_addiupc(DisasContext *ctx, int rx, int imm,
>                    int is_64_bit, int extended)
>   {
> -    TCGv t0;
> -
>       if (extended && (ctx->hflags & MIPS_HFLAG_BMASK)) {
>           gen_reserved_instruction(ctx);
>           return;
>       }
>   
> -    t0 = tcg_temp_new();
> -
> -    tcg_gen_movi_tl(t0, pc_relative_pc(ctx));
> -    tcg_gen_addi_tl(cpu_gpr[rx], t0, imm);
> +    tcg_gen_addi_tl(cpu_gpr[rx], tcg_constant_tl(pc_relative_pc(ctx)), imm);
>       if (!is_64_bit) {
>           tcg_gen_ext32s_tl(cpu_gpr[rx], cpu_gpr[rx]);
>       }

Oh dear.  All of this can be computed during translate.

     target_ulong val = pc_relative_pc(ctx) + imm;
     if (!is_64_bit) {
         val = (int32_t)val;
     }
     tcg_gen_movi_tl(cpu_gpr[rx], val);


> diff --git a/target/mips/tcg/micromips_translate.c.inc b/target/mips/tcg/micromips_translate.c.inc
> index 171508f7deb..d044592f83e 100644
> --- a/target/mips/tcg/micromips_translate.c.inc
> +++ b/target/mips/tcg/micromips_translate.c.inc
> @@ -980,8 +980,7 @@ static void gen_ldst_pair(DisasContext *ctx, uint32_t opc, int rd,
>           tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, mo_endian(ctx) | MO_SL |
>                              ctx->default_tcg_memop_mask);
>           gen_store_gpr(t1, rd);
> -        tcg_gen_movi_tl(t1, 4);
> -        gen_op_addr_add(ctx, t0, t0, t1);
> +        gen_op_addr_add(ctx, t0, t0, tcg_constant_tl(4));

It might be useful to add a gen_op_addr_addi() helper.

Anyway, it's all an incremental improvement so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~