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