Re: [PATCH v3 13/20] target/s390x: Drop free_compare

[Ilya Leoshkevich]

On Sat, 2023-03-04 at 10:18 -0800, Richard Henderson wrote:
> Translators are no longer required to free tcg temporaries.
> Remove the g1 and g2 members of DisasCompare, as they were
> used to track which temps needed to be freed.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: qemu-s390x@nongnu.org
> ---
>  target/s390x/tcg/translate.c | 46 +---------------------------------
> --
>  1 file changed, 1 insertion(+), 45 deletions(-)
> 
> diff --git a/target/s390x/tcg/translate.c
> b/target/s390x/tcg/translate.c
> index 811049ea28..76a1233946 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -156,8 +156,6 @@ struct DisasContext {
>  typedef struct {
>      TCGCond cond:8;
>      bool is_64;
> -    bool g1;
> -    bool g2;
>      union {
>          struct { TCGv_i64 a, b; } s64;
>          struct { TCGv_i32 a, b; } s32;
> @@ -722,7 +720,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>          c->cond = (mask ? TCG_COND_ALWAYS : TCG_COND_NEVER);
>          c->u.s32.a = cc_op;
>          c->u.s32.b = cc_op;
> -        c->g1 = c->g2 = true;
>          c->is_64 = false;
>          return;
>      }
> @@ -839,7 +836,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>  
>      /* Load up the arguments of the comparison.  */
>      c->is_64 = true;
> -    c->g1 = c->g2 = false;
>      switch (old_cc_op) {
>      case CC_OP_LTGT0_32:
>          c->is_64 = false;
> @@ -861,13 +857,11 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>      case CC_OP_FLOGR:
>          c->u.s64.a = cc_dst;
>          c->u.s64.b = tcg_constant_i64(0);
> -        c->g1 = true;
>          break;
>      case CC_OP_LTGT_64:
>      case CC_OP_LTUGTU_64:
>          c->u.s64.a = cc_src;
>          c->u.s64.b = cc_dst;
> -        c->g1 = c->g2 = true;
>          break;
>  
>      case CC_OP_TM_32:
> @@ -882,7 +876,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>      case CC_OP_SUBU:
>          c->is_64 = true;
>          c->u.s64.b = tcg_constant_i64(0);
> -        c->g1 = true;
>          switch (mask) {
>          case 8 | 2:
>          case 4 | 1: /* result */
> @@ -900,7 +893,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>      case CC_OP_STATIC:
>          c->is_64 = false;
>          c->u.s32.a = cc_op;
> -        c->g1 = true;
>          switch (mask) {
>          case 0x8 | 0x4 | 0x2: /* cc != 3 */
>              cond = TCG_COND_NE;
> @@ -916,7 +908,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>              break;
>          case 0x8 | 0x2: /* cc == 0 || cc == 2 => (cc & 1) == 0 */
>              cond = TCG_COND_EQ;
> -            c->g1 = false;
>              c->u.s32.a = tcg_temp_new_i32();
>              c->u.s32.b = tcg_constant_i32(0);
>              tcg_gen_andi_i32(c->u.s32.a, cc_op, 1);
> @@ -935,7 +926,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>              break;
>          case 0x4 | 0x1: /* cc == 1 || cc == 3 => (cc & 1) != 0 */
>              cond = TCG_COND_NE;
> -            c->g1 = false;
>              c->u.s32.a = tcg_temp_new_i32();
>              c->u.s32.b = tcg_constant_i32(0);
>              tcg_gen_andi_i32(c->u.s32.a, cc_op, 1);
> @@ -959,7 +949,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>          default:
>              /* CC is masked by something else: (8 >> cc) & mask.  */
>              cond = TCG_COND_NE;
> -            c->g1 = false;
>              c->u.s32.a = tcg_temp_new_i32();
>              c->u.s32.b = tcg_constant_i32(0);
>              tcg_gen_shr_i32(c->u.s32.a, tcg_constant_i32(8), cc_op);
> @@ -974,24 +963,6 @@ static void disas_jcc(DisasContext *s,
> DisasCompare *c, uint32_t mask)
>      c->cond = cond;
>  }
>  
> -static void free_compare(DisasCompare *c)
> -{
> -    if (!c->g1) {
> -        if (c->is_64) {
> -            tcg_temp_free_i64(c->u.s64.a);
> -        } else {
> -            tcg_temp_free_i32(c->u.s32.a);
> -        }
> -    }
> -    if (!c->g2) {
> -        if (c->is_64) {
> -            tcg_temp_free_i64(c->u.s64.b);
> -        } else {
> -            tcg_temp_free_i32(c->u.s32.b);
> -        }
> -    }
> -}
> -
>  /*
> =====================================================================
> = */
>  /* Define the insn format enumeration.  */
>  #define F0(N)                         FMT_##N,
> @@ -1302,7 +1273,6 @@ static DisasJumpType help_branch(DisasContext
> *s, DisasCompare *c,
>      }
>  
>   egress:
> -    free_compare(c);
>      return ret;
>  }
>  
> @@ -1612,8 +1582,6 @@ static DisasJumpType op_bct32(DisasContext *s,
> DisasOps *o)
>  
>      c.cond = TCG_COND_NE;
>      c.is_64 = false;
> -    c.g1 = false;
> -    c.g2 = false;
>  
>      t = tcg_temp_new_i64();
>      tcg_gen_subi_i64(t, regs[r1], 1);
> @@ -1635,8 +1603,6 @@ static DisasJumpType op_bcth(DisasContext *s,
> DisasOps *o)
>  
>      c.cond = TCG_COND_NE;
>      c.is_64 = false;
> -    c.g1 = false;
> -    c.g2 = false;
>  
>      t = tcg_temp_new_i64();
>      tcg_gen_shri_i64(t, regs[r1], 32);
> @@ -1659,8 +1625,6 @@ static DisasJumpType op_bct64(DisasContext *s,
> DisasOps *o)
>  
>      c.cond = TCG_COND_NE;
>      c.is_64 = true;
> -    c.g1 = true;
> -    c.g2 = false;
>  
>      tcg_gen_subi_i64(regs[r1], regs[r1], 1);
>      c.u.s64.a = regs[r1];
> @@ -1680,8 +1644,6 @@ static DisasJumpType op_bx32(DisasContext *s,
> DisasOps *o)
>  
>      c.cond = (s->insn->data ? TCG_COND_LE : TCG_COND_GT);
>      c.is_64 = false;
> -    c.g1 = false;
> -    c.g2 = false;
>  
>      t = tcg_temp_new_i64();
>      tcg_gen_add_i64(t, regs[r1], regs[r3]);
> @@ -1708,15 +1670,12 @@ static DisasJumpType op_bx64(DisasContext *s,
> DisasOps *o)
>  
>      if (r1 == (r3 | 1)) {
>          c.u.s64.b = load_reg(r3 | 1);
> -        c.g2 = false;
>      } else {
>          c.u.s64.b = regs[r3 | 1];
> -        c.g2 = true;
>      }
>  
>      tcg_gen_add_i64(regs[r1], regs[r1], regs[r3]);
>      c.u.s64.a = regs[r1];
> -    c.g1 = true;
>  
>      return help_branch(s, &c, is_imm, imm, o->in2);
>  }
> @@ -1731,7 +1690,7 @@ static DisasJumpType op_cj(DisasContext *s,
> DisasOps *o)
>      if (s->insn->data) {
>          c.cond = tcg_unsigned_cond(c.cond);
>      }
> -    c.is_64 = c.g1 = c.g2 = true;
> +    c.is_64 = true;
>      c.u.s64.a = o->in1;
>      c.u.s64.b = o->in2;
>  
> @@ -2925,13 +2884,11 @@ static DisasJumpType op_loc(DisasContext *s,
> DisasOps *o)
>      if (c.is_64) {
>          tcg_gen_movcond_i64(c.cond, o->out, c.u.s64.a, c.u.s64.b,
>                              o->in2, o->in1);
> -        free_compare(&c);
>      } else {
>          TCGv_i32 t32 = tcg_temp_new_i32();
>          TCGv_i64 t, z;
>  
>          tcg_gen_setcond_i32(c.cond, t32, c.u.s32.a, c.u.s32.b);
> -        free_compare(&c);
>  
>          t = tcg_temp_new_i64();
>          tcg_gen_extu_i32_i64(t, t32);
> @@ -4022,7 +3979,6 @@ static DisasJumpType op_soc(DisasContext *s,
> DisasOps *o)
>      } else {
>          tcg_gen_brcond_i32(c.cond, c.u.s32.a, c.u.s32.b, lab);
>      }
> -    free_compare(&c);
>  
>      r1 = get_field(s, r1);
>      a = get_address(s, 0, get_field(s, b2), get_field(s, d2));

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>