Re: [PATCH v2 6/8] target/ppc: Move div/mod fixed-point insns (64 bits operands) to decodetree.

[Richard Henderson]

On 5/17/24 14:48, Nicholas Piggin wrote:
> On Tue Apr 23, 2024 at 4:32 PM AEST, Chinmay Rath wrote:
> > Moving the below instructions to decodetree specification :
> >
> >         divd[u, e, eu][o][.]    : XO-form
> >         mod{sd, ud}             : X-form
> >
> > With this patch, all the fixed-point arithmetic instructions have been
> > moved to decodetree.
> > The changes were verified by validating that the tcg ops generated by those
> > instructions remain the same, which were captured using the '-d in_asm,op' flag.
> > Also, remaned do_divwe method in fixedpoint-impl.c.inc to do_dive because it is
> > now used to divide doubleword operands as well, and not just words.
> >
> > Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> [...]
>
> > +static bool do_divd(DisasContext *ctx, arg_XO *a, bool sign)
> > +{
> > +    gen_op_arith_divd(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
> > +                      sign, a->oe, a->rc);
> > +    return true;
> > +}
> > +
> > +static bool do_modd(DisasContext *ctx, arg_X *a, bool sign)
> > +{
> > +    REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> > +    gen_op_arith_modd(ctx, cpu_gpr[a->rt], cpu_gpr[a->ra], cpu_gpr[a->rb],
> > +                      sign);
> > +    return true;
> > +}
> > +
> > +TRANS64(DIVD, do_divd, true);
> > +TRANS64(DIVDU, do_divd, false);
> > +TRANS64(DIVDE, do_dive, gen_helper_DIVDE);
> > +TRANS64(DIVDEU, do_dive, gen_helper_DIVDEU);
> > +
> > +TRANS64(MODSD, do_modd, true);
> > +TRANS64(MODUD, do_modd, false);
>
> Sigh. I'm having to fix a bunch of these for 32-bit builds. Just
> doing the #ifdef TARGET_PPC64 ... #else qemu_build_not_reached();
> thing.
>
> Which is quite ugly and actually prevents using some of these
> macros and requires open coding (e.g., because DIVDE helper is
> not declared for 32-bit in this case).

Compare sparc:

# define gen_helper_pdist     ({ qemu_build_not_reached(); NULL; })

etc.

> Maybe we should move 64-bit only instructions into their own
> .decode file and not build them for 32-bit, so we don't have
> to add all these dummy translate functions for them.

That's another option, yes.  The decodetree script will take multiple input files to 
produce one output, so you could separate the insns by base vs 64-bit.


r~