Re: [PATCH v11 06/26] target/loongarch: Add fixed point bit instruction translation

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

On 11/19/21 7:13 AM, Song Gao wrote:
> +static bool gen_rr(DisasContext *ctx, arg_rr *a,
> +                   DisasExtend src_ext, DisasExtend dst_ext,
> +                   void (*func)(TCGv, TCGv))
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd, dst_ext);
> +    TCGv src1 = gpr_src(ctx, a->rj, src_ext);
> +
> +    func(dest, src1);
> +
> +    if (dst_ext) {
> +        gen_set_gpr(a->rd, dest, dst_ext);
> +    }

Again, I think you should call gen_set_gpr unconditionally.

> +static bool trans_bytepick_w(DisasContext *ctx, arg_bytepick_w *a)
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
> +    TCGv src2 = gpr_src(ctx, a->rk, EXT_NONE);
> +
> +    tcg_gen_concat_tl_i64(dest, src1, src2);
> +    tcg_gen_sextract_i64(dest, dest, (32 - (a->sa) * 8), 32);
> +
> +    return true;
> +}

Better to use gen_rrr_sa.

> +static bool trans_bytepick_d(DisasContext *ctx, arg_bytepick_d *a)
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
> +    TCGv src2 = gpr_src(ctx, a->rk, EXT_NONE);
> +
> +    tcg_gen_extract2_i64(dest, src1, src2, (64 - (a->sa) * 8));
> +    return true;
> +}

Likewise.

> +static void gen_ctz_w(TCGv dest, TCGv src1)
> +{
> +    tcg_gen_ori_tl(dest, src1, (target_ulong)MAKE_64BIT_MASK(32, 32));
> +    tcg_gen_ctzi_tl(dest, dest, 32);

This should be TARGET_LONG_BITS.  It will never happen, because the value is not zero per 
the OR, but it's what is most efficient for a tcg backend that naturally produces 
TARGET_LONG_BITS for a TL-sized ctz.

> +}
> +
> +static void gen_cto_w(TCGv dest, TCGv src1)
> +{
> +    tcg_gen_not_tl(dest, src1);
> +    tcg_gen_ext32u_tl(dest, dest);
> +    gen_ctz_w(dest, dest);
> +}

The EXT32U here is useless, as the OR within gen_ctz_w overrides it.

> +&rr_2bw       rd rj msbw lsbw
> +&rr_2bd       rd rj msbd lsbd

Merge these.


r~