[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 1/2] ppc: Add support for 'mffscrn', 'mffscrni'
From: |
Richard Henderson |
Subject: |
Re: [Qemu-ppc] [PATCH v2 1/2] ppc: Add support for 'mffscrn', 'mffscrni' instructions |
Date: |
Tue, 17 Sep 2019 13:45:35 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
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...
> -#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.
Possibly FP_MODE should itself be removed. It's used
exactly once, in mffsl, which could just as easily
reference FP_RN | FP_DRN.
r~