Re: [PATCH v3 16/22] target/ppc: Move dtstdc[q]/dtstdg[q] to decodetree

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

On 9/10/21 4:26 AM, Luis Pires wrote:
> +&Z22_bf_fra     bf fra dm
> +@Z22_bf_fra     ...... bf:3 .. fra:5 dm:6 ......... .           &Z22_bf_fra
> +
> +%z22_frap       17:4 !function=times_2
> +@Z22_bf_frap    ...... bf:3 .. ....0 dm:6 ......... .           &Z22_bf_fra fra=%z22_frap

How confusing.  There's a typo in the manual for these insns, with the minor opcode (XO) 
field at the wrong location.  It's correct in the summary of instruction formats at the 
beginning of the manual.

> -#define GEN_DFP_BF_A_DCM(name)                    \
> -static void gen_##name(DisasContext *ctx)         \
> -{                                                 \
> -    TCGv_ptr ra;                                  \
> -    TCGv_i32 dcm;                                 \
> -    if (unlikely(!ctx->fpu_enabled)) {            \
> -        gen_exception(ctx, POWERPC_EXCP_FPU);     \
> -        return;                                   \
> -    }                                             \
> -    gen_update_nip(ctx, ctx->base.pc_next - 4);   \
> -    ra = gen_fprp_ptr(rA(ctx->opcode));           \
> -    dcm = tcg_const_i32(DCM(ctx->opcode));        \
> -    gen_helper_##name(cpu_crf[crfD(ctx->opcode)], \
> -                      cpu_env, ra, dcm);          \
> -    tcg_temp_free_ptr(ra);                        \
> -    tcg_temp_free_i32(dcm);                       \
> +#define TRANS_DFP_BF_A_DCM(NAME)                             \
> +static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a)   \
> +{                                                            \
> +    TCGv_ptr ra;                                             \
> +    REQUIRE_INSNS_FLAGS2(ctx, DFP);                          \
> +    REQUIRE_FPU(ctx);                                        \
> +    ra = gen_fprp_ptr(a->fra);                               \
> +    gen_helper_##NAME(cpu_crf[a->bf],                        \
> +                      cpu_env, ra, tcg_constant_i32(a->dm)); \
> +    tcg_temp_free_ptr(ra);                                   \
> +    return true;                                             \
>  }

Functional change: you're no longer storing nip.  It does seem wrong, but that fix should 
be broken out to a separate patch.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~