Re: [Qemu-devel] [PATCH v2 1/2] ppc: Add support for 'mffscrn', 'mffscrni' instructions

[Paul Clarke <pc@us.ibm.com>]

On 9/17/19 3:45 PM, Richard Henderson wrote:
> On 9/16/19 1:02 PM, Paul A. Clarke wrote:
>> +#define FP_DRN2         (1ull << FPSCR_DRN2)
>> +#define FP_DRN1         (1ull << FPSCR_DRN1)
>> +#define FP_DRN0         (1ull << FPSCR_DRN0)
>> +#define FP_DRN          (FP_DRN2 | FP_DRN1 | FP_DRN0)
> 
> Why not just 7ull << FPSCR_DRN?
> Are the individual DRN bits separately useful?
> They don't appear to be...

I was just following what was done with RN:
#define FPSCR_RN1    1
#define FPSCR_RN0    0  /* Floating-point rounding control                   */
...
#define FP_RN1          (1ull << FPSCR_RN1)
#define FP_RN0          (1ull << FPSCR_RN0)
#define FP_RN           (FP_RN1 | FP_RN0)

>> -#define FP_MODE         FP_RN
>> +#define FP_MODE         (FP_DRN | FP_RN)
> 
> This, I think, isn't helpful.
> 
>> +static void gen_helper_mffscrn(DisasContext *ctx, TCGv_i64 t1)
>> +{
>> +    TCGv_i64 t0 = tcg_temp_new_i64();
>> +    TCGv_i32 mask = tcg_const_i32(0x0001);
>> +
>> +    gen_reset_fpstatus();
>> +    tcg_gen_extu_tl_i64(t0, cpu_fpscr);
>> +    tcg_gen_andi_i64(t0, t0, FP_MODE | FP_ENABLES);
>> +    set_fpr(rD(ctx->opcode), t0);
>> +
>> +    /* Mask FPSCR value to clear RN.  */
>> +    tcg_gen_andi_i64(t0, t0, ~FP_MODE);
> 
> Because here,
> 
>> +static void gen_mffscrn(DisasContext *ctx)
>> +{
>> +    TCGv_i64 t1;
>> +
>> +    if (unlikely(!(ctx->insns_flags2 & PPC2_ISA300))) {
>> +        return gen_mffs(ctx);
>> +    }
>> +
>> +    if (unlikely(!ctx->fpu_enabled)) {
>> +        gen_exception(ctx, POWERPC_EXCP_FPU);
>> +        return;
>> +    }
>> +
>> +    t1 = tcg_temp_new_i64();
>> +    get_fpr(t1, rB(ctx->opcode));
>> +    /* Mask FRB to get just RN.  */
>> +    tcg_gen_andi_i64(t1, t1, FP_MODE);
> 
> and here, we're only interested in RN, not DRN.

Oh, you're right, of course.

> Possibly FP_MODE should itself be removed.  It's used
> exactly once, in mffsl, which could just as easily
> reference FP_RN | FP_DRN.

I will do, and then send an updated version.

PC