Re: [PATCH 04/24] target/arm: Use tcg_gen_negsetcond_*

[Peter Maydell]

On Tue, 8 Aug 2023 at 04:15, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-a64.c | 22 +++++++++-------------
>  target/arm/tcg/translate.c     | 12 ++++--------
>  2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index 5fa1257d32..ac16593699 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -4935,9 +4935,12 @@ static void disas_cond_select(DisasContext *s, 
> uint32_t insn)
>
>      if (rn == 31 && rm == 31 && (else_inc ^ else_inv)) {
>          /* CSET & CSETM.  */
> -        tcg_gen_setcond_i64(tcg_invert_cond(c.cond), tcg_rd, c.value, zero);
>          if (else_inv) {
> -            tcg_gen_neg_i64(tcg_rd, tcg_rd);
> +            tcg_gen_negsetcond_i64(tcg_invert_cond(c.cond),
> +                                   tcg_rd, c.value, zero);
> +        } else {
> +            tcg_gen_setcond_i64(tcg_invert_cond(c.cond),
> +                                tcg_rd, c.value, zero);
>          }
>      } else {
>          TCGv_i64 t_true = cpu_reg(s, rn);
> @@ -8670,13 +8673,10 @@ static void handle_3same_64(DisasContext *s, int 
> opcode, bool u,
>          }
>          break;
>      case 0x6: /* CMGT, CMHI */
> -        /* 64 bit integer comparison, result = test ? (2^64 - 1) : 0.
> -         * We implement this using setcond (test) and then negating.
> -         */
>          cond = u ? TCG_COND_GTU : TCG_COND_GT;
>      do_cmop:
> -        tcg_gen_setcond_i64(cond, tcg_rd, tcg_rn, tcg_rm);
> -        tcg_gen_neg_i64(tcg_rd, tcg_rd);
> +        /* 64 bit integer comparison, result = test ? -1 : 0. */
> +        tcg_gen_negsetcond_i64(cond, tcg_rd, tcg_rn, tcg_rm);
>          break;
>      case 0x7: /* CMGE, CMHS */
>          cond = u ? TCG_COND_GEU : TCG_COND_GE;
> @@ -9265,14 +9265,10 @@ static void handle_2misc_64(DisasContext *s, int 
> opcode, bool u,
>          }
>          break;
>      case 0xa: /* CMLT */
> -        /* 64 bit integer comparison against zero, result is
> -         * test ? (2^64 - 1) : 0. We implement via setcond(!test) and
> -         * subtracting 1.
> -         */
> +        /* 64 bit integer comparison against zero, result is test ? 1 : 0. */

surely "-1" ?

>          cond = TCG_COND_LT;
>      do_cmop:
> -        tcg_gen_setcondi_i64(cond, tcg_rd, tcg_rn, 0);
> -        tcg_gen_neg_i64(tcg_rd, tcg_rd);
> +        tcg_gen_negsetcond_i64(cond, tcg_rd, tcg_rn, tcg_constant_i64(0));
>          break;
>      case 0x8: /* CMGT, CMGE */
>          cond = u ? TCG_COND_GE : TCG_COND_GT;
> diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
> index b71ac2d0d5..31d3130e4c 100644
> --- a/target/arm/tcg/translate.c
> +++ b/target/arm/tcg/translate.c
> @@ -2946,13 +2946,11 @@ void gen_gvec_sqrdmlsh_qc(unsigned vece, uint32_t 
> rd_ofs, uint32_t rn_ofs,
>  #define GEN_CMP0(NAME, COND)                                            \
>      static void gen_##NAME##0_i32(TCGv_i32 d, TCGv_i32 a)               \
>      {                                                                   \
> -        tcg_gen_setcondi_i32(COND, d, a, 0);                            \
> -        tcg_gen_neg_i32(d, d);                                          \
> +        tcg_gen_negsetcond_i32(COND, d, a, tcg_constant_i32(0));        \
>      }                                                                   \
>      static void gen_##NAME##0_i64(TCGv_i64 d, TCGv_i64 a)               \
>      {                                                                   \
> -        tcg_gen_setcondi_i64(COND, d, a, 0);                            \
> -        tcg_gen_neg_i64(d, d);                                          \
> +        tcg_gen_negsetcond_i64(COND, d, a, tcg_constant_i64(0));        \
>      }                                                                   \
>      static void gen_##NAME##0_vec(unsigned vece, TCGv_vec d, TCGv_vec a) \
>      {                                                                   \
> @@ -3863,15 +3861,13 @@ void gen_gvec_mls(unsigned vece, uint32_t rd_ofs, 
> uint32_t rn_ofs,
>  static void gen_cmtst_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b)
>  {
>      tcg_gen_and_i32(d, a, b);
> -    tcg_gen_setcondi_i32(TCG_COND_NE, d, d, 0);
> -    tcg_gen_neg_i32(d, d);
> +    tcg_gen_negsetcond_i32(TCG_COND_NE, d, d, tcg_constant_i32(0));
>  }
>
>  void gen_cmtst_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
>  {
>      tcg_gen_and_i64(d, a, b);
> -    tcg_gen_setcondi_i64(TCG_COND_NE, d, d, 0);
> -    tcg_gen_neg_i64(d, d);
> +    tcg_gen_negsetcond_i64(TCG_COND_NE, d, d, tcg_constant_i64(0));
>  }
>
>  static void gen_cmtst_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM