Re: [PATCH v11 04/26] target/loongarch: Add fixed point arithmetic instruction translation

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

On 11/19/21 7:13 AM, Song Gao wrote:
> +static bool gen_rrr(DisasContext *ctx, arg_rrr *a,
> +                    DisasExtend src1_ext, DisasExtend src2_ext,
> +                    DisasExtend dst_ext, void (*func)(TCGv, TCGv, TCGv))
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd, dst_ext);
> +    TCGv src1 = gpr_src(ctx, a->rj, src1_ext);
> +    TCGv src2 = gpr_src(ctx, a->rk, src2_ext);
> +
> +    func(dest, src1, src2);
> +
> +    /* dst_ext is EXT_NONE and input is dest, We don't run gen_set_gpr. */
> +    if (dst_ext) {
> +        gen_set_gpr(a->rd, dest, dst_ext);
> +    }

Why the (incomplete) condition around gen_set_gpr?

I think it's a bug to not name EXT_NONE in the test (just because EXT_NONE == 0 now...), 
but I also think you should not have added the test at all.  We will not generate any code 
in the end within gen_set_gpr, but it allows the routines to be self-contained.  You 
shouldn't assume what gpr_dst returned.


r~