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

[Song Gao]

Hi, Richard.

On 07/26/2021 11:53 PM, Richard Henderson wrote:
> 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:
>>        
>> 20210709042608.883256-1-richard.henderson@linaro.org">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)
> 
> 
OK

Again, thank you kindly help.

Thanks
Song Gao.