diff mbox series

[v2,1/2] ppc: Add support for 'mffscrn', 'mffscrni' instructions

Message ID 1568653336-16819-1-git-send-email-pc@us.ibm.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] ppc: Add support for 'mffscrn', 'mffscrni' instructions | expand

Commit Message

Paul A. Clarke Sept. 16, 2019, 5:02 p.m. UTC 
From: "Paul A. Clarke" <pc@us.ibm.com>

ISA 3.0B added a set of Floating-Point Status and Control Register (FPSCR)
instructions: mffsce, mffscdrn, mffscdrni, mffscrn, mffscrni, mffsl.
This patch adds support for 'mffscrn' and 'mffscrni' instructions.

'mffscrn' and 'mffscrni' are similar to 'mffsl', except they do not return
the status bits (FI, FR, FPRF) and they also set the rounding mode in the
FPSCR.

On CPUs without support for 'mffscrn'/'mffscrni' (below ISA 3.0), the
instructions will execute identically to 'mffs'.

Signed-off-by: Paul A. Clarke <pc@us.ibm.com>
---
v2:
- Add DRN to returned FPSCR value.
- Add DRN defines to target/ppc/cpu.h.

 target/ppc/cpu.h                   | 10 +++++-
 target/ppc/dfp_helper.c            |  2 +-
 target/ppc/internal.h              |  3 ++
 target/ppc/translate/fp-impl.inc.c | 67 ++++++++++++++++++++++++++++++++++++++
 target/ppc/translate/fp-ops.inc.c  |  4 +++
 5 files changed, 84 insertions(+), 2 deletions(-)

Comments

Richard Henderson Sept. 17, 2019, 8:45 p.m. UTC | #1 
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~
Paul A. Clarke Sept. 17, 2019, 9:49 p.m. UTC | #2 
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
diff mbox series

Patch

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index eaee1a5..dcea2f0 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -559,6 +559,9 @@  enum {
 
 /*****************************************************************************/
 /* Floating point status and control register                                */
+#define FPSCR_DRN2   34 /* Decimal Floating-Point rounding control           */
+#define FPSCR_DRN1   33 /* Decimal Floating-Point rounding control           */
+#define FPSCR_DRN0   32 /* Decimal Floating-Point rounding control           */
 #define FPSCR_FX     31 /* Floating-point exception summary                  */
 #define FPSCR_FEX    30 /* Floating-point enabled exception summary          */
 #define FPSCR_VX     29 /* Floating-point invalid operation exception summ.  */
@@ -592,6 +595,7 @@  enum {
 #define FPSCR_NI     2  /* Floating-point non-IEEE mode                      */
 #define FPSCR_RN1    1
 #define FPSCR_RN0    0  /* Floating-point rounding control                   */
+#define fpscr_drn    (((env->fpscr) & FP_DRN) >> FPSCR_DRN0)
 #define fpscr_fex    (((env->fpscr) >> FPSCR_FEX)    & 0x1)
 #define fpscr_vx     (((env->fpscr) >> FPSCR_VX)     & 0x1)
 #define fpscr_ox     (((env->fpscr) >> FPSCR_OX)     & 0x1)
@@ -627,6 +631,10 @@  enum {
 #define fpscr_eex (((env->fpscr) >> FPSCR_XX) & ((env->fpscr) >> FPSCR_XE) &  \
                    0x1F)
 
+#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)
 #define FP_FX           (1ull << FPSCR_FX)
 #define FP_FEX          (1ull << FPSCR_FEX)
 #define FP_VX           (1ull << FPSCR_VX)
@@ -662,7 +670,7 @@  enum {
 #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)
 #define FP_ENABLES      (FP_VE | FP_OE | FP_UE | FP_ZE | FP_XE)
 #define FP_STATUS       (FP_FR | FP_FI | FP_FPRF)
 
diff --git a/target/ppc/dfp_helper.c b/target/ppc/dfp_helper.c
index f102177..da8e08a 100644
--- a/target/ppc/dfp_helper.c
+++ b/target/ppc/dfp_helper.c
@@ -48,7 +48,7 @@  static void dfp_prepare_rounding_mode(decContext *context, uint64_t fpscr)
 {
     enum rounding rnd;
 
-    switch ((fpscr >> 32) & 0x7) {
+    switch ((fpscr & FP_DRN) >> FPSCR_DRN0) {
     case 0:
         rnd = DEC_ROUND_HALF_EVEN;
         break;
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index d3d327e..15d655b 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -157,6 +157,9 @@  EXTRACT_HELPER(FPL, 25, 1);
 EXTRACT_HELPER(FPFLM, 17, 8);
 EXTRACT_HELPER(FPW, 16, 1);
 
+/* mffscrni */
+EXTRACT_HELPER(RM, 11, 2);
+
 /* addpcis */
 EXTRACT_HELPER_SPLIT_3(DX, 10, 6, 6, 5, 16, 1, 1, 0, 0)
 #if defined(TARGET_PPC64)
diff --git a/target/ppc/translate/fp-impl.inc.c b/target/ppc/translate/fp-impl.inc.c
index 7cd9d8d..59a4faf 100644
--- a/target/ppc/translate/fp-impl.inc.c
+++ b/target/ppc/translate/fp-impl.inc.c
@@ -639,6 +639,73 @@  static void gen_mffsl(DisasContext *ctx)
     tcg_temp_free_i64(t0);
 }
 
+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);
+
+    /* Merge RN into FPSCR value.  */
+    tcg_gen_or_i64(t0, t0, t1);
+
+    gen_helper_store_fpscr(cpu_env, t0, mask);
+
+    tcg_temp_free_i32(mask);
+    tcg_temp_free_i64(t0);
+}
+
+/* mffscrn */
+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);
+
+    gen_helper_mffscrn(ctx, t1);
+
+    tcg_temp_free_i64(t1);
+}
+
+/* mffscrni */
+static void gen_mffscrni(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_const_i64((uint64_t)RM(ctx->opcode));
+
+    gen_helper_mffscrn(ctx, t1);
+
+    tcg_temp_free_i64(t1);
+}
+
 /* mtfsb0 */
 static void gen_mtfsb0(DisasContext *ctx)
 {
diff --git a/target/ppc/translate/fp-ops.inc.c b/target/ppc/translate/fp-ops.inc.c
index 88ebc25..f2bcf0e 100644
--- a/target/ppc/translate/fp-ops.inc.c
+++ b/target/ppc/translate/fp-ops.inc.c
@@ -107,6 +107,10 @@  GEN_HANDLER(mcrfs, 0x3F, 0x00, 0x02, 0x0063F801, PPC_FLOAT),
 GEN_HANDLER_E_2(mffs, 0x3F, 0x07, 0x12, 0x00, 0x00000000, PPC_FLOAT, PPC_NONE),
 GEN_HANDLER_E_2(mffsl, 0x3F, 0x07, 0x12, 0x18, 0x00000000, PPC_FLOAT,
     PPC2_ISA300),
+GEN_HANDLER_E_2(mffscrn, 0x3F, 0x07, 0x12, 0x16, 0x00000000, PPC_FLOAT,
+    PPC_NONE),
+GEN_HANDLER_E_2(mffscrni, 0x3F, 0x07, 0x12, 0x17, 0x00000000, PPC_FLOAT,
+    PPC_NONE),
 GEN_HANDLER(mtfsb0, 0x3F, 0x06, 0x02, 0x001FF800, PPC_FLOAT),
 GEN_HANDLER(mtfsb1, 0x3F, 0x06, 0x01, 0x001FF800, PPC_FLOAT),
 GEN_HANDLER(mtfsf, 0x3F, 0x07, 0x16, 0x00000000, PPC_FLOAT),