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:
>>
>> https://patchew.org/QEMU/20210709042608.883256-1-richard.hender...@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.
