Re: [PATCH v2 07/22] target/loongarch: Add fixed point arithmetic instruction translation

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

On 7/26/21 1:56 AM, Song Gao wrote:
> Hi, Richard.
> 
> On 07/23/2021 08:46 AM, Richard Henderson wrote:
>> On 7/20/21 11:53 PM, Song Gao wrote:
>>> +/* Fixed point arithmetic operation instruction translation */
>>> +static bool trans_add_w(DisasContext *ctx, arg_add_w *a)
>>> +{
>>> +    TCGv Rd = cpu_gpr[a->rd];
>>> +    TCGv Rj = cpu_gpr[a->rj];
>>> +    TCGv Rk = cpu_gpr[a->rk];
>>> +
>>> +    if (a->rd == 0) {
>>> +        /* Nop */
>>> +        return true;
>>> +    }
>>> +
>>> +    if (a->rj != 0 && a->rk != 0) {
>>> +        tcg_gen_add_tl(Rd, Rj, Rk);
>>> +        tcg_gen_ext32s_tl(Rd, Rd);
>>> +    } else if (a->rj == 0 && a->rk != 0) {
>>> +        tcg_gen_mov_tl(Rd, Rk);
>>> +    } else if (a->rj != 0 && a->rk == 0) {
>>> +        tcg_gen_mov_tl(Rd, Rj);
>>> +    } else {
>>> +        tcg_gen_movi_tl(Rd, 0);
>>> +    }
>>> +
>>> +    return true;
>>> +}
>>
>> Do not do all of this "if reg(n) zero" testing.
>>
>> Use a common function to perform the gpr lookup, and a small callback function for the operation.  Often, the callback function already exists within include/tcg/tcg-op.h.
>>
>> Please see my riscv cleanup patch set I referenced vs patch 6.
> 
> I am not sure  that 'riscv cleanup' patchs at:
>    
>     https://patchew.org/QEMU/20210709042608.883256-1-richard.henderson@linaro.org
> 
> It seems that  gpr_dst/gpr_src are common function to perform the gpr lookup. is that right?

More than that.  The gen_arith() function, for example, performs all of the bookkeeping 
for a binary operation.

For example,

static bool gen_arith(DisasContext *ctx, arg_fmt_rdrjrk *a,
                       void (*func)(TCGv, TCGv, TCGv))
{
    TCGv dest = gpr_dst(ctx, a->rd);
    TCGv src1 = gpr_src(ctx, a->rj);
    TCGv src2 = gpr_src(ctx, a->rk);

     func(dest, src1, src2);
     return true;
}

#define TRANS(NAME, FUNC, ...) \
     static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
     { return FUNC(ctx, a, __VA_ARGS__); }

static void gen_add_w(TCGv dest, TCGv src1, TCGv src2)
{
     tcg_gen_add_tl(dest, src1, src2);
     tcg_gen_ext32s_tl(dest, dest);
}

TRANS(add_w, gen_arith, gen_add_w)
TRANS(add_d, gen_arith, tcg_gen_add_tl)


r~