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

[Richard Henderson]

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)


r~