Re: [PATCH 37/67] target/arm: Convert CLS, CLZ (vector) to decodetree

[Peter Maydell]

On Sun, 1 Dec 2024 at 15:10, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/tcg/translate-a64.c | 37 ++++++++++++++++------------------
>  target/arm/tcg/a64.decode      |  2 ++
>  2 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index 4abc786cf6..312bf48020 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -8920,6 +8920,20 @@ static bool do_gvec_fn2(DisasContext *s, arg_qrr_e *a, 
> GVecGen2Fn *fn)
>  TRANS(ABS_v, do_gvec_fn2, a, tcg_gen_gvec_abs)
>  TRANS(NEG_v, do_gvec_fn2, a, tcg_gen_gvec_neg)
>
> +static bool do_gvec_fn2_bhs(DisasContext *s, arg_qrr_e *a, GVecGen2Fn *fn)
> +{
> +    if (a->esz == MO_64) {
> +        return false;
> +    }
> +    if (fp_access_check(s)) {
> +        gen_gvec_fn2(s, a->q, a->rd, a->rn, fn, a->esz);
> +    }
> +    return true;
> +}
> +
> +TRANS(CLS_v, do_gvec_fn2_bhs, a, gen_gvec_cls)
> +TRANS(CLZ_v, do_gvec_fn2_bhs, a, gen_gvec_clz)
> +
>  /* Common vector code for handling integer to FP conversion */
>  static void handle_simd_intfp_conv(DisasContext *s, int rd, int rn,
>                                     int elements, int is_signed,
> @@ -9219,13 +9233,6 @@ static void handle_2misc_64(DisasContext *s, int 
> opcode, bool u,
>      TCGCond cond;
>
>      switch (opcode) {
> -    case 0x4: /* CLS, CLZ */
> -        if (u) {
> -            tcg_gen_clzi_i64(tcg_rd, tcg_rn, 64);
> -        } else {
> -            tcg_gen_clrsb_i64(tcg_rd, tcg_rn);
> -        }
> -        break;
>      case 0x5: /* NOT */

This was dead code, right? We only call handle_2misc_64()
for size == 3, but size == 3 is an unallocated encoding for
"CLS/CLZ (vector)", which only deals with elements sizes up
to 32 bits. Worth mentioning in the commit message, I think.

(I was wondering why the new code doesn't have any cases for
operating on 64-bit elements whereas this old code did seem
to handle it.)

>          /* This opcode is shared with CNT and RBIT but we have earlier
>           * enforced that size == 3 if and only if this is the NOT insn.

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

thanks
-- PMM