[Qemu-devel] [PATCH v2 00/12] target/mips: Amend R5900 support

[Qemu-devel] [PATCH v2 00/12] target/mips: Amend R5900 support

[Fredrik Noring]

This series amends the R5900 support with the following noncritical
features:

- R5900 MFLO1, MFHI1, MTLO1 and MTHI1 are generated in gen_HILO1_tx79.

- R5900 DIV1 and DIVU1 are generated in gen_div1_tx79.

- The R5900 LQ and SQ instructions are now also covered by the Toshiba
  MMI ASE, as per the TX79 manual[1].

- The three-operand MADD and MADDU instructions specific to the R5900
  and the Toshiba TX19, TX39 and TX79 cores are now supported and tested
  by the R5900 TCG test suite.

- The three-operand MADD1 and MADDU1 pipeline 1 instructions specific
  to the R5900 and the Toshiba TX79 core are now supported and tested
  by the R5900 TCG test suite.

- The membership field of struct mips_opcode is now uint64_t instead
  of unsigned long, that is too small in 32-bit builds.

- R5900 disassembly constants are defined.

- The R5900 instructions DIV1, DIVU1, MFLO, MTLO, MFHI, MTHI, MULT1 and
  MULTU1 are now disassembled. Unfortunately, the opcodes for MADD1 and
  MADDU1 clash with the opcodes for CLZ and CLO, resulting in incorrect
  disassembly. MADD1 and MADDU1 are therefore left undefined.

This series has been successfully built with the 8 different build
configurations

    {gcc,clang} x -m64 x mips{,64}el-{linux-user,softmmu}

in addition successfully completing the R5900 test suite

    cd tests/tcg/mips/mipsr5900 && make check

Reference:

[1] "Toshiba TX System RISC TX79 Core Architecture", Toshiba Corporation,
    section B.3.2, p. B-4, <https://wiki.qemu.org/File:C790.pdf>.

Changes in v2:
- Drop rejected rename of ASE_MMI to ASE_TOSHIBA_MMI
- Generate R5900 DIV1 and DIVU1 in gen_div1_tx79
- Generate R5900 MFLO1, MFHI1, MTLO1 and MTHI1 in gen_HILO1_tx79

Fredrik Noring (10):
  target/mips: Generate R5900 MFLO1, MFHI1, MTLO1 and MTHI1 in gen_HILO1_tx79
  target/mips: Generate R5900 DIV1 and DIVU1 in gen_div1_tx79
  target/mips: R5900 LQ and SQ also belong to the Toshiba MMI ASE
  target/mips: Support R5900 three-operand MADD1 and MADDU1
  tests/tcg/mips: Test R5900 three-operand MADD
  tests/tcg/mips: Test R5900 three-operand MADD1
  tests/tcg/mips: Test R5900 three-operand MADDU
  tests/tcg/mips: Test R5900 three-operand MADDU1
  disas/mips: Define R5900 disassembly constants
  disas/mips: Disassemble R5900 DIV[U]1, M{F,T}{LO,HI}1 and MULT[U]1

Philippe Mathieu-Daudé (2):
  target/mips: Support Toshiba specific three-operand MADD and MADDU
  disas/mips: Increase 'member of ISAs' flag holder size

 disas/mips.c                      |  22 +++-
 target/mips/translate.c           | 206 ++++++++++++++++++++++++++----
 tests/tcg/mips/mipsr5900/Makefile |   2 +
 tests/tcg/mips/mipsr5900/madd.c   |  78 +++++++++++
 tests/tcg/mips/mipsr5900/maddu.c  |  70 ++++++++++
 5 files changed, 351 insertions(+), 27 deletions(-)
 create mode 100644 tests/tcg/mips/mipsr5900/madd.c
 create mode 100644 tests/tcg/mips/mipsr5900/maddu.c

-- 
2.18.1

[Qemu-devel] [PATCH v2 01/12] target/mips: Generate R5900 MFLO1, MFHI1, MTLO1 and MTHI1 in gen_HILO1_tx79

[Fredrik Noring]

MFLO1, MFHI1, MTLO1 and MTHI1 are generated in gen_HILO1_tx79 instead of
the generic gen_HILO.

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/translate.c | 67 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 11 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 60320cbe69..f3993cf7d7 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4359,24 +4359,72 @@ static void gen_shift(DisasContext *ctx, uint32_t opc,
     tcg_temp_free(t1);
 }
 
+/* Move to and from TX79 HI1/LO1 registers. */
+static void gen_HILO1_tx79(DisasContext *ctx, uint32_t opc, int reg)
+{
+    if (reg == 0 && (opc == TX79_MMI_MFHI1 || opc == TX79_MMI_MFLO1)) {
+        /* Treat as NOP. */
+        return;
+    }
+
+    switch (opc) {
+    case TX79_MMI_MFHI1:
+#if defined(TARGET_MIPS64)
+        tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[1]);
+#else
+        tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[1]);
+#endif
+        break;
+    case TX79_MMI_MFLO1:
+#if defined(TARGET_MIPS64)
+        tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_LO[1]);
+#else
+        tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[1]);
+#endif
+        break;
+    case TX79_MMI_MTHI1:
+        if (reg != 0) {
+#if defined(TARGET_MIPS64)
+            tcg_gen_ext32s_tl(cpu_HI[1], cpu_gpr[reg]);
+#else
+            tcg_gen_mov_tl(cpu_HI[1], cpu_gpr[reg]);
+#endif
+        } else {
+            tcg_gen_movi_tl(cpu_HI[1], 0);
+        }
+        break;
+    case TX79_MMI_MTLO1:
+        if (reg != 0) {
+#if defined(TARGET_MIPS64)
+            tcg_gen_ext32s_tl(cpu_LO[1], cpu_gpr[reg]);
+#else
+            tcg_gen_mov_tl(cpu_LO[1], cpu_gpr[reg]);
+#endif
+        } else {
+            tcg_gen_movi_tl(cpu_LO[1], 0);
+        }
+        break;
+    default:
+        MIPS_INVAL("MFTHILO TX79");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    }
+}
+
 /* Arithmetic on HI/LO registers */
 static void gen_HILO(DisasContext *ctx, uint32_t opc, int acc, int reg)
 {
-    if (reg == 0 && (opc == OPC_MFHI || opc == TX79_MMI_MFHI1 ||
-                     opc == OPC_MFLO || opc == TX79_MMI_MFLO1)) {
+    if (reg == 0 && (opc == OPC_MFHI || opc == OPC_MFLO)) {
         /* Treat as NOP. */
         return;
     }
 
     if (acc != 0) {
-        if (!(ctx->insn_flags & INSN_R5900)) {
-            check_dsp(ctx);
-        }
+        check_dsp(ctx);
     }
 
     switch (opc) {
     case OPC_MFHI:
-    case TX79_MMI_MFHI1:
 #if defined(TARGET_MIPS64)
         if (acc != 0) {
             tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[acc]);
@@ -4387,7 +4435,6 @@ static void gen_HILO(DisasContext *ctx, uint32_t opc, int acc, int reg)
         }
         break;
     case OPC_MFLO:
-    case TX79_MMI_MFLO1:
 #if defined(TARGET_MIPS64)
         if (acc != 0) {
             tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_LO[acc]);
@@ -4398,7 +4445,6 @@ static void gen_HILO(DisasContext *ctx, uint32_t opc, int acc, int reg)
         }
         break;
     case OPC_MTHI:
-    case TX79_MMI_MTHI1:
         if (reg != 0) {
 #if defined(TARGET_MIPS64)
             if (acc != 0) {
@@ -4413,7 +4459,6 @@ static void gen_HILO(DisasContext *ctx, uint32_t opc, int acc, int reg)
         }
         break;
     case OPC_MTLO:
-    case TX79_MMI_MTLO1:
         if (reg != 0) {
 #if defined(TARGET_MIPS64)
             if (acc != 0) {
@@ -26500,11 +26545,11 @@ static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
         break;
     case TX79_MMI_MTLO1:
     case TX79_MMI_MTHI1:
-        gen_HILO(ctx, opc, 1, rs);
+        gen_HILO1_tx79(ctx, opc, rs);
         break;
     case TX79_MMI_MFLO1:
     case TX79_MMI_MFHI1:
-        gen_HILO(ctx, opc, 1, rd);
+        gen_HILO1_tx79(ctx, opc, rd);
         break;
     case TX79_MMI_MADD:          /* TODO: TX79_MMI_MADD */
     case TX79_MMI_MADDU:         /* TODO: TX79_MMI_MADDU */
-- 
2.18.1

[Qemu-devel] [PATCH v2 02/12] target/mips: Generate R5900 DIV1 and DIVU1 in gen_div1_tx79

[Fredrik Noring]

DIV1 and DIVU1 are generated in gen_div1_tx79 instead of the generic
gen_muldiv.

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/translate.c | 65 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index f3993cf7d7..6e5a8a2565 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4759,6 +4759,63 @@ static void gen_r6_muldiv(DisasContext *ctx, int opc, int rd, int rs, int rt)
     tcg_temp_free(t1);
 }
 
+static void gen_div1_tx79(DisasContext *ctx, uint32_t opc, int rs, int rt)
+{
+    TCGv t0, t1;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+
+    gen_load_gpr(t0, rs);
+    gen_load_gpr(t1, rt);
+
+    switch (opc) {
+    case TX79_MMI_DIV1:
+        {
+            TCGv t2 = tcg_temp_new();
+            TCGv t3 = tcg_temp_new();
+            tcg_gen_ext32s_tl(t0, t0);
+            tcg_gen_ext32s_tl(t1, t1);
+            tcg_gen_setcondi_tl(TCG_COND_EQ, t2, t0, INT_MIN);
+            tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, -1);
+            tcg_gen_and_tl(t2, t2, t3);
+            tcg_gen_setcondi_tl(TCG_COND_EQ, t3, t1, 0);
+            tcg_gen_or_tl(t2, t2, t3);
+            tcg_gen_movi_tl(t3, 0);
+            tcg_gen_movcond_tl(TCG_COND_NE, t1, t2, t3, t2, t1);
+            tcg_gen_div_tl(cpu_LO[1], t0, t1);
+            tcg_gen_rem_tl(cpu_HI[1], t0, t1);
+            tcg_gen_ext32s_tl(cpu_LO[1], cpu_LO[1]);
+            tcg_gen_ext32s_tl(cpu_HI[1], cpu_HI[1]);
+            tcg_temp_free(t3);
+            tcg_temp_free(t2);
+        }
+        break;
+    case TX79_MMI_DIVU1:
+        {
+            TCGv t2 = tcg_const_tl(0);
+            TCGv t3 = tcg_const_tl(1);
+            tcg_gen_ext32u_tl(t0, t0);
+            tcg_gen_ext32u_tl(t1, t1);
+            tcg_gen_movcond_tl(TCG_COND_EQ, t1, t1, t2, t3, t1);
+            tcg_gen_divu_tl(cpu_LO[1], t0, t1);
+            tcg_gen_remu_tl(cpu_HI[1], t0, t1);
+            tcg_gen_ext32s_tl(cpu_LO[1], cpu_LO[1]);
+            tcg_gen_ext32s_tl(cpu_HI[1], cpu_HI[1]);
+            tcg_temp_free(t3);
+            tcg_temp_free(t2);
+        }
+        break;
+    default:
+        MIPS_INVAL("div1 TX79");
+        generate_exception_end(ctx, EXCP_RI);
+        goto out;
+    }
+ out:
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 static void gen_muldiv(DisasContext *ctx, uint32_t opc,
                        int acc, int rs, int rt)
 {
@@ -4771,14 +4828,11 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
     gen_load_gpr(t1, rt);
 
     if (acc != 0) {
-        if (!(ctx->insn_flags & INSN_R5900)) {
-            check_dsp(ctx);
-        }
+        check_dsp(ctx);
     }
 
     switch (opc) {
     case OPC_DIV:
-    case TX79_MMI_DIV1:
         {
             TCGv t2 = tcg_temp_new();
             TCGv t3 = tcg_temp_new();
@@ -4800,7 +4854,6 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
         }
         break;
     case OPC_DIVU:
-    case TX79_MMI_DIVU1:
         {
             TCGv t2 = tcg_const_tl(0);
             TCGv t3 = tcg_const_tl(1);
@@ -26541,7 +26594,7 @@ static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
         break;
     case TX79_MMI_DIV1:
     case TX79_MMI_DIVU1:
-        gen_muldiv(ctx, opc, 1, rs, rt);
+        gen_div1_tx79(ctx, opc, rs, rt);
         break;
     case TX79_MMI_MTLO1:
     case TX79_MMI_MTHI1:
-- 
2.18.1

[Qemu-devel] [PATCH v2 03/12] target/mips: R5900 LQ and SQ also belong to the Toshiba MMI ASE

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/translate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 6e5a8a2565..624e53644d 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27992,7 +27992,8 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case OPC_SPECIAL3:
-        if (ctx->insn_flags & INSN_R5900) {
+        if ((ctx->insn_flags & INSN_R5900) &&
+            (ctx->insn_flags & ASE_MMI)) {
             decode_tx79_sq(env, ctx);    /* TX79_SQ */
         } else {
             decode_opc_special3(env, ctx);
@@ -28656,7 +28657,8 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case OPC_MSA: /* OPC_MDMX */
-        if (ctx->insn_flags & INSN_R5900) {
+        if ((ctx->insn_flags & INSN_R5900) &&
+            (ctx->insn_flags & ASE_MMI)) {
             decode_tx79_lq(env, ctx);    /* TX79_LQ */
         } else {
             /* MDMX: Not implemented. */
-- 
2.18.1

[Qemu-devel] [PATCH v2 04/12] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Fredrik Noring]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The three-operand MADD and MADDU are specific to the
Toshiba TX19/TX39/TX79 cores.

The "32-Bit TX System RISC TX39 Family Architecture manual"
is available at https://wiki.qemu.org/File:DSAE0022432.pdf

Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
Signed-off-by: Fredrik Noring <noring@nocrew.org>
Tested-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/translate.c | 58 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 624e53644d..4808cb49c3 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -5009,8 +5009,8 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
 }
 
 /*
- * These MULT and MULTU instructions implemented in for example the
- * Toshiba/Sony R5900 and the Toshiba TX19, TX39 and TX79 core
+ * These MULT[U] and MADD[U] instructions implemented in for example
+ * the Toshiba/Sony R5900 and the Toshiba TX19, TX39 and TX79 core
  * architectures are special three-operand variants with the syntax
  *
  *     MULT[U][1] rd, rs, rt
@@ -5019,6 +5019,14 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
  *
  *     (rd, LO, HI) <- rs * rt
  *
+ * and
+ *
+ *     MADD[U]    rd, rs, rt
+ *
+ * such that
+ *
+ *     (rd, LO, HI) <- (LO, HI) + rs * rt
+ *
  * where the low-order 32-bits of the result is placed into both the
  * GPR rd and the special register LO. The high-order 32-bits of the
  * result is placed into the special register HI.
@@ -5075,8 +5083,48 @@ static void gen_mul_txx9(DisasContext *ctx, uint32_t opc,
             tcg_temp_free_i32(t3);
         }
         break;
+    case TX79_MMI_MADD:
+        {
+            TCGv_i64 t2 = tcg_temp_new_i64();
+            TCGv_i64 t3 = tcg_temp_new_i64();
+
+            tcg_gen_ext_tl_i64(t2, t0);
+            tcg_gen_ext_tl_i64(t3, t1);
+            tcg_gen_mul_i64(t2, t2, t3);
+            tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]);
+            tcg_gen_add_i64(t2, t2, t3);
+            tcg_temp_free_i64(t3);
+            gen_move_low32(cpu_LO[acc], t2);
+            gen_move_high32(cpu_HI[acc], t2);
+            if (rd) {
+                gen_move_low32(cpu_gpr[rd], t2);
+            }
+            tcg_temp_free_i64(t2);
+        }
+        break;
+    case TX79_MMI_MADDU:
+        {
+            TCGv_i64 t2 = tcg_temp_new_i64();
+            TCGv_i64 t3 = tcg_temp_new_i64();
+
+            tcg_gen_ext32u_tl(t0, t0);
+            tcg_gen_ext32u_tl(t1, t1);
+            tcg_gen_extu_tl_i64(t2, t0);
+            tcg_gen_extu_tl_i64(t3, t1);
+            tcg_gen_mul_i64(t2, t2, t3);
+            tcg_gen_concat_tl_i64(t3, cpu_LO[acc], cpu_HI[acc]);
+            tcg_gen_add_i64(t2, t2, t3);
+            tcg_temp_free_i64(t3);
+            gen_move_low32(cpu_LO[acc], t2);
+            gen_move_high32(cpu_HI[acc], t2);
+            if (rd) {
+                gen_move_low32(cpu_gpr[rd], t2);
+            }
+            tcg_temp_free_i64(t2);
+        }
+        break;
     default:
-        MIPS_INVAL("mul TXx9");
+        MIPS_INVAL("mul/madd TXx9");
         generate_exception_end(ctx, EXCP_RI);
         goto out;
     }
@@ -26590,6 +26638,8 @@ static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
         break;
     case TX79_MMI_MULT1:
     case TX79_MMI_MULTU1:
+    case TX79_MMI_MADD:
+    case TX79_MMI_MADDU:
         gen_mul_txx9(ctx, opc, rd, rs, rt);
         break;
     case TX79_MMI_DIV1:
@@ -26604,8 +26654,6 @@ static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
     case TX79_MMI_MFHI1:
         gen_HILO1_tx79(ctx, opc, rd);
         break;
-    case TX79_MMI_MADD:          /* TODO: TX79_MMI_MADD */
-    case TX79_MMI_MADDU:         /* TODO: TX79_MMI_MADDU */
     case TX79_MMI_PLZCW:         /* TODO: TX79_MMI_PLZCW */
     case TX79_MMI_MADD1:         /* TODO: TX79_MMI_MADD1 */
     case TX79_MMI_MADDU1:        /* TODO: TX79_MMI_MADDU1 */
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 04/12] target/mips: Support Toshiba specific three-operand MADD and MADDU

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Subject: [PATCH v2 04/12] target/mips: Support Toshiba specific three-operand MADD and MADDU
>
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

This patch is selected for integration in the next MIPS pull request scheduled shortly.

[Qemu-devel] [PATCH v2 05/12] target/mips: Support R5900 three-operand MADD1 and MADDU1

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/translate.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 4808cb49c3..57b17ad8f6 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -5021,7 +5021,7 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
  *
  * and
  *
- *     MADD[U]    rd, rs, rt
+ *     MADD[U][1] rd, rs, rt
  *
  * such that
  *
@@ -5083,6 +5083,9 @@ static void gen_mul_txx9(DisasContext *ctx, uint32_t opc,
             tcg_temp_free_i32(t3);
         }
         break;
+    case TX79_MMI_MADD1:
+        acc = 1;
+        /* Fall through */
     case TX79_MMI_MADD:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
@@ -5102,6 +5105,9 @@ static void gen_mul_txx9(DisasContext *ctx, uint32_t opc,
             tcg_temp_free_i64(t2);
         }
         break;
+    case TX79_MMI_MADDU1:
+        acc = 1;
+        /* Fall through */
     case TX79_MMI_MADDU:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
@@ -26640,6 +26646,8 @@ static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
     case TX79_MMI_MULTU1:
     case TX79_MMI_MADD:
     case TX79_MMI_MADDU:
+    case TX79_MMI_MADD1:
+    case TX79_MMI_MADDU1:
         gen_mul_txx9(ctx, opc, rd, rs, rt);
         break;
     case TX79_MMI_DIV1:
@@ -26655,8 +26663,6 @@ static void decode_tx79_mmi(CPUMIPSState *env, DisasContext *ctx)
         gen_HILO1_tx79(ctx, opc, rd);
         break;
     case TX79_MMI_PLZCW:         /* TODO: TX79_MMI_PLZCW */
-    case TX79_MMI_MADD1:         /* TODO: TX79_MMI_MADD1 */
-    case TX79_MMI_MADDU1:        /* TODO: TX79_MMI_MADDU1 */
     case TX79_MMI_PMFHL:         /* TODO: TX79_MMI_PMFHL */
     case TX79_MMI_PMTHL:         /* TODO: TX79_MMI_PMTHL */
     case TX79_MMI_PSLLH:         /* TODO: TX79_MMI_PSLLH */
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 05/12] target/mips: Support R5900 three-operand MADD1 and MADDU1

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Subject: [PATCH v2 05/12] target/mips: Support R5900 three-operand MADD1 and MADDU1

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

This patch is selected for integration in the next MIPS pull request scheduled shortly.

[Qemu-devel] [PATCH v2 06/12] tests/tcg/mips: Test R5900 three-operand MADD

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 tests/tcg/mips/mipsr5900/Makefile |  1 +
 tests/tcg/mips/mipsr5900/madd.c   | 45 +++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 tests/tcg/mips/mipsr5900/madd.c

diff --git a/tests/tcg/mips/mipsr5900/Makefile b/tests/tcg/mips/mipsr5900/Makefile
index a1c388bc3c..97ca2a671c 100644
--- a/tests/tcg/mips/mipsr5900/Makefile
+++ b/tests/tcg/mips/mipsr5900/Makefile
@@ -10,6 +10,7 @@ CFLAGS  = -Wall -mabi=32 -march=r5900 -static
 
 TESTCASES = div1.tst
 TESTCASES += divu1.tst
+TESTCASES += madd.tst
 TESTCASES += mflohi1.tst
 TESTCASES += mtlohi1.tst
 TESTCASES += mult.tst
diff --git a/tests/tcg/mips/mipsr5900/madd.c b/tests/tcg/mips/mipsr5900/madd.c
new file mode 100644
index 0000000000..9ad2ea6dbb
--- /dev/null
+++ b/tests/tcg/mips/mipsr5900/madd.c
@@ -0,0 +1,45 @@
+/*
+ * Test R5900-specific three-operand MADD.
+ */
+
+#include <stdio.h>
+#include <inttypes.h>
+#include <assert.h>
+
+int64_t madd(int64_t a, int32_t rs, int32_t rt)
+{
+    int32_t lo = a;
+    int32_t hi = a >> 32;
+    int32_t rd;
+    int64_t r;
+
+    __asm__ __volatile__ (
+            "    mtlo %5\n"
+            "    mthi %6\n"
+            "    madd %0, %3, %4\n"
+            "    mflo %1\n"
+            "    mfhi %2\n"
+            : "=r" (rd), "=r" (lo), "=r" (hi)
+            : "r" (rs), "r" (rt), "r" (lo), "r" (hi));
+    r = ((int64_t)hi << 32) | (uint32_t)lo;
+
+    assert(a + (int64_t)rs * rt == r);
+    assert(rd == lo);
+
+    return r;
+}
+
+static void verify_madd(int64_t a, int32_t rs, int32_t rt, int64_t expected)
+{
+    assert(madd(a, rs, rt) == expected);
+    assert(madd(a, -rs, rt) == a + a - expected);
+    assert(madd(a, rs, -rt) == a + a - expected);
+    assert(madd(a, -rs, -rt) == expected);
+}
+
+int main()
+{
+    verify_madd(13, 17, 19, 336);
+
+    return 0;
+}
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 06/12] tests/tcg/mips: Test R5900 three-operand MADD

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Subject: [PATCH v2 06/12] tests/tcg/mips: Test R5900 three-operand MADD

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

This patch is selected for integration in the next MIPS pull request scheduled shortly.

[Qemu-devel] [PATCH v2 07/12] tests/tcg/mips: Test R5900 three-operand MADD1

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 tests/tcg/mips/mipsr5900/madd.c | 43 +++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/tests/tcg/mips/mipsr5900/madd.c b/tests/tcg/mips/mipsr5900/madd.c
index 9ad2ea6dbb..f6f215e1c3 100644
--- a/tests/tcg/mips/mipsr5900/madd.c
+++ b/tests/tcg/mips/mipsr5900/madd.c
@@ -1,5 +1,5 @@
 /*
- * Test R5900-specific three-operand MADD.
+ * Test R5900-specific three-operand MADD and MADD1.
  */
 
 #include <stdio.h>
@@ -29,12 +29,45 @@ int64_t madd(int64_t a, int32_t rs, int32_t rt)
     return r;
 }
 
+int64_t madd1(int64_t a, int32_t rs, int32_t rt)
+{
+    int32_t lo = a;
+    int32_t hi = a >> 32;
+    int32_t rd;
+    int64_t r;
+
+    __asm__ __volatile__ (
+            "    mtlo1 %5\n"
+            "    mthi1 %6\n"
+            "    madd1 %0, %3, %4\n"
+            "    mflo1 %1\n"
+            "    mfhi1 %2\n"
+            : "=r" (rd), "=r" (lo), "=r" (hi)
+            : "r" (rs), "r" (rt), "r" (lo), "r" (hi));
+    r = ((int64_t)hi << 32) | (uint32_t)lo;
+
+    assert(a + (int64_t)rs * rt == r);
+    assert(rd == lo);
+
+    return r;
+}
+
+static int64_t madd_variants(int64_t a, int32_t rs, int32_t rt)
+{
+    int64_t rd  = madd(a, rs, rt);
+    int64_t rd1 = madd1(a, rs, rt);
+
+    assert(rd == rd1);
+
+    return rd;
+}
+
 static void verify_madd(int64_t a, int32_t rs, int32_t rt, int64_t expected)
 {
-    assert(madd(a, rs, rt) == expected);
-    assert(madd(a, -rs, rt) == a + a - expected);
-    assert(madd(a, rs, -rt) == a + a - expected);
-    assert(madd(a, -rs, -rt) == expected);
+    assert(madd_variants(a, rs, rt) == expected);
+    assert(madd_variants(a, -rs, rt) == a + a - expected);
+    assert(madd_variants(a, rs, -rt) == a + a - expected);
+    assert(madd_variants(a, -rs, -rt) == expected);
 }
 
 int main()
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 07/12] tests/tcg/mips: Test R5900 three-operand MADD1

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Subject: [PATCH v2 07/12] tests/tcg/mips: Test R5900 three-operand MADD1

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

This patch is selected for integration in the next MIPS pull request scheduled shortly.

[Qemu-devel] [PATCH v2 08/12] tests/tcg/mips: Test R5900 three-operand MADDU

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 tests/tcg/mips/mipsr5900/Makefile |  1 +
 tests/tcg/mips/mipsr5900/maddu.c  | 37 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 tests/tcg/mips/mipsr5900/maddu.c

diff --git a/tests/tcg/mips/mipsr5900/Makefile b/tests/tcg/mips/mipsr5900/Makefile
index 97ca2a671c..27ee5d5f54 100644
--- a/tests/tcg/mips/mipsr5900/Makefile
+++ b/tests/tcg/mips/mipsr5900/Makefile
@@ -11,6 +11,7 @@ CFLAGS  = -Wall -mabi=32 -march=r5900 -static
 TESTCASES = div1.tst
 TESTCASES += divu1.tst
 TESTCASES += madd.tst
+TESTCASES += maddu.tst
 TESTCASES += mflohi1.tst
 TESTCASES += mtlohi1.tst
 TESTCASES += mult.tst
diff --git a/tests/tcg/mips/mipsr5900/maddu.c b/tests/tcg/mips/mipsr5900/maddu.c
new file mode 100644
index 0000000000..e4e552102d
--- /dev/null
+++ b/tests/tcg/mips/mipsr5900/maddu.c
@@ -0,0 +1,37 @@
+/*
+ * Test R5900-specific three-operand MADDU.
+ */
+
+#include <stdio.h>
+#include <inttypes.h>
+#include <assert.h>
+
+uint64_t maddu(uint64_t a, uint32_t rs, uint32_t rt)
+{
+    uint32_t lo = a;
+    uint32_t hi = a >> 32;
+    uint32_t rd;
+    uint64_t r;
+
+    __asm__ __volatile__ (
+            "    mtlo  %5\n"
+            "    mthi  %6\n"
+            "    maddu %0, %3, %4\n"
+            "    mflo  %1\n"
+            "    mfhi  %2\n"
+            : "=r" (rd), "=r" (lo), "=r" (hi)
+            : "r" (rs), "r" (rt), "r" (lo), "r" (hi));
+    r = ((uint64_t)hi << 32) | (uint32_t)lo;
+
+    assert(a + (uint64_t)rs * rt == r);
+    assert(rd == lo);
+
+    return r;
+}
+
+int main()
+{
+    assert(maddu(13, 17, 19) == 336);
+
+    return 0;
+}
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 08/12] tests/tcg/mips: Test R5900 three-operand MADDU

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Subject: [PATCH v2 08/12] tests/tcg/mips: Test R5900 three-operand MADDU

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

This patch is selected for integration in the next MIPS pull request scheduled shortly.

[Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 tests/tcg/mips/mipsr5900/maddu.c | 37 ++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/mips/mipsr5900/maddu.c b/tests/tcg/mips/mipsr5900/maddu.c
index e4e552102d..30936fb2b4 100644
--- a/tests/tcg/mips/mipsr5900/maddu.c
+++ b/tests/tcg/mips/mipsr5900/maddu.c
@@ -1,5 +1,5 @@
 /*
- * Test R5900-specific three-operand MADDU.
+ * Test R5900-specific three-operand MADDU and MADDU1.
  */
 
 #include <stdio.h>
@@ -29,9 +29,42 @@ uint64_t maddu(uint64_t a, uint32_t rs, uint32_t rt)
     return r;
 }
 
+uint64_t maddu1(uint64_t a, uint32_t rs, uint32_t rt)
+{
+    uint32_t lo = a;
+    uint32_t hi = a >> 32;
+    uint32_t rd;
+    uint64_t r;
+
+    __asm__ __volatile__ (
+            "    mtlo1  %5\n"
+            "    mthi1  %6\n"
+            "    maddu1 %0, %3, %4\n"
+            "    mflo1  %1\n"
+            "    mfhi1  %2\n"
+            : "=r" (rd), "=r" (lo), "=r" (hi)
+            : "r" (rs), "r" (rt), "r" (lo), "r" (hi));
+    r = ((uint64_t)hi << 32) | (uint32_t)lo;
+
+    assert(a + (uint64_t)rs * rt == r);
+    assert(rd == lo);
+
+    return r;
+}
+
+static int64_t maddu_variants(int64_t a, int32_t rs, int32_t rt)
+{
+    int64_t rd  = maddu(a, rs, rt);
+    int64_t rd1 = maddu1(a, rs, rt);
+
+    assert(rd == rd1);
+
+    return rd;
+}
+
 int main()
 {
-    assert(maddu(13, 17, 19) == 336);
+    assert(maddu_variants(13, 17, 19) == 336);
 
     return 0;
 }
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Subject: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

This patch is selected for integration in the next MIPS pull request scheduled shortly.

THANKS FOR ALL EFFORTS!

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Fredrik Noring]

Thanks Aleksandar!

> > From: Fredrik Noring <noring@nocrew.org>
> > Subject: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1
> 
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> This patch is selected for integration in the next MIPS pull request scheduled shortly.
> 
> THANKS FOR ALL EFFORTS!

Is this a preparation to revert commit 823f2897bdd7 ("target/mips: Disable
R5900 support")?

Fredrik

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Tuesday, January 1, 2019 7:27 PM
> To: Aleksandar Markovic
> Cc: Aurelien Jarno; Philippe Mathieu-Daudé; Jürgen Urban; Maciej W. Rozycki;
> qemu-devel@nongnu.org
> Subject: Re: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1
> 
> Thanks Aleksandar!
> 
> > > From: Fredrik Noring <noring@nocrew.org>
> > > Subject: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1
> >
> > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> >
> > This patch is selected for integration in the next MIPS pull request scheduled shortly.
> >
> > THANKS FOR ALL EFFORTS!
> 
> Is this a preparation to revert commit 823f2897bdd7 ("target/mips: Disable
> R5900 support")?
> 
> Fredrik

Hello!

Glad to see you back!

Yes, one can say this is a step towards reenabling R5900 support.

However, this is not the only step. I will let you know about all
needed details in near future - and I apologize for being little
slow with responses so far, I was being swamped with other tasks.

Let's not rush. If one learned that the rush is bad, that is me,
making severe missteps, overly eager to do the thing sooner rather
than later. However, the faster in not always the better.

At this moment I have a question a suggestion for you:

- Question: Do you have somewhere link to n32 R5900 toolchain, or
similar thing that would enable me to test R5900's n32 in QEMU
(I'll do the tweaks in QEMU by myself for that, but I need a way
to compile R5900 test programs for n32)?

- Suggestion: The next MIPS pull request is scehuled for Friday,
Jan 18, 2018. It would be fantastic if you could prepare the
following by Jan 14:

  * Add 32 TCGv_i64 registers that would represent higher halves
  of R5900 general purpose registers.
  * Add TCGv_i32 register SA (shift amount).
  * Perhaps consider adding higher halves of registers HI an LO
  independently on HI/LO array used by DSP.
  * It is customary to implement R/W access while introducing
  such registers:
    * Implement R/W access instructions to higher halves of
    R5900 GPRs:
      * LQ
      * SQ
    * Implement R/W access instructions to SA register:
      * MFSA
      * MTSA
      * MTSAH
      * MTSAB

I think a reasonable person would consider that the number and
size of registers of emulated CPU is a fundamental thing that
must be done before enabling its support - hence my suggestion
above.

If you don't have enough time resources before Jan 14, that is
fine too, do everything at your own pace.

My vision is that we continue step by step amending R5900 until
we reach a decent and stable state of emulation and than enable
the R5900 support for end user. (I'll provide all the details
later on.)

Let me know if you have questions and/or different opinion.

Looking forward to seeing you!

Aleksandar

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Fredrik Noring]

Hi Aleksandar,

> Glad to see you back!

Likewise!

> Yes, one can say this is a step towards reenabling R5900 support.

Great!

> At this moment I have a question a suggestion for you:
> 
> - Question: Do you have somewhere link to n32 R5900 toolchain, or
> similar thing that would enable me to test R5900's n32 in QEMU

Yes. The only (small) changes needed for n32 are related to Glibc,
since the R5900 does not implement DMULT etc. in hardware. The attached
patch (see below) traps these instructions, but the Glibc patch proposal
(not yet submitted) will instead emulate them, which is believed to be
faster (with actual hardware; with QEMU it would most likely instead be
significantly slower since QEMU would need to emulate this emulation).

You will also need commit d728eb9085d8 ("MIPS: Default to --with-llsc for
the R5900 Linux target as well"), made for o32 and GCC, as explained in

https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg03649.html

unless you compile GCC from HEAD that already has it.

> (I'll do the tweaks in QEMU by myself for that, but I need a way
> to compile R5900 test programs for n32)?

I just verified that the only change needed with QEMU apart from reverting
commit 823f2897bdd7 ("target/mips: Disable R5900 support") is this:

--- a/linux-user/mips64/target_elf.h
+++ b/linux-user/mips64/target_elf.h
@@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_64R6) {
         return "I6400";
     }
+    if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
+        return "R5900";
+    }
     return "5KEf";
 }
 #endif

> - Suggestion: The next MIPS pull request is scehuled for Friday,
> Jan 18, 2018. It would be fantastic if you could prepare the
> following by Jan 14:
> 
>   * Add 32 TCGv_i64 registers that would represent higher halves
>   of R5900 general purpose registers.
>   * Add TCGv_i32 register SA (shift amount).
>   * Perhaps consider adding higher halves of registers HI an LO
>   independently on HI/LO array used by DSP.
>   * It is customary to implement R/W access while introducing
>   such registers:
>     * Implement R/W access instructions to higher halves of
>     R5900 GPRs:
>       * LQ
>       * SQ
>     * Implement R/W access instructions to SA register:
>       * MFSA
>       * MTSA
>       * MTSAH
>       * MTSAB

Sounds good except I'm not sure it can be done before 14 January!

> I think a reasonable person would consider that the number and
> size of registers of emulated CPU is a fundamental thing that
> must be done before enabling its support - hence my suggestion
> above.

Well, no package in any popular Linux distribution require these R5900
extensions, or as far as I know, try to make use of them. So with an
initial use case of running a Linux distribution in user mode, support
for them is certainly not needed at all.

Of course, support would be needed if anyone would like to start
implementing R5900 specific extensions. Also, the Linux kernel needs a
few of those instructions, so implementing them would make it possible
to run the kernel in system mode, which would be very useful too.

> If you don't have enough time resources before Jan 14, that is
> fine too, do everything at your own pace.

Thanks!

> My vision is that we continue step by step amending R5900 until
> we reach a decent and stable state of emulation and than enable
> the R5900 support for end user. (I'll provide all the details
> later on.)

OK.

Fredrik

--- a/sysdeps/mips/mips64/addmul_1.S
+++ b/sysdeps/mips/mips64/addmul_1.S
@@ -33,6 +33,9 @@
 	.option pic2
 #endif
 ENTRY (__mpn_addmul_1)
+#ifdef _MIPS_ARCH_R5900
+	.set	mips3
+#endif
 #ifdef PIC
 	SETUP_GP /* ??? unused */
 #endif
diff --git a/sysdeps/mips/mips64/mul_1.S b/sysdeps/mips/mips64/mul_1.S
index 8707257a68..19e3ae5eae 100644
--- a/sysdeps/mips/mips64/mul_1.S
+++ b/sysdeps/mips/mips64/mul_1.S
@@ -34,6 +34,9 @@
 	.option pic2
 #endif
 ENTRY (__mpn_mul_1)
+#ifdef _MIPS_ARCH_R5900
+	.set	mips3
+#endif
 #ifdef __PIC__
 	SETUP_GP /* ??? unused */
 #endif
diff --git a/sysdeps/mips/mips64/submul_1.S b/sysdeps/mips/mips64/submul_1.S
index fb9a1c2375..763c5bb687 100644
--- a/sysdeps/mips/mips64/submul_1.S
+++ b/sysdeps/mips/mips64/submul_1.S
@@ -34,6 +34,9 @@
 	.option pic2
 #endif
 ENTRY (__mpn_submul_1)
+#ifdef _MIPS_ARCH_R5900
+	.set	mips3
+#endif
 #ifdef __PIC__
 	SETUP_GP /* ??? unused */
 #endif
diff --git a/sysdeps/mips/sys/tas.h b/sysdeps/mips/sys/tas.h
index d5ed013e28..ad797bfb1d 100644
--- a/sysdeps/mips/sys/tas.h
+++ b/sysdeps/mips/sys/tas.h
@@ -38,11 +38,14 @@ __NTH (_test_and_set (int *__p, int __v))
 {
   int __r, __t;
 
+  /* The R5900 reports itself as MIPS III but it does not have LL/SC.  */
   __asm__ __volatile__
     ("/* Inline test and set */\n"
      ".set	push\n\t"
 #if _MIPS_SIM == _ABIO32 && __mips < 2
      ".set	mips2\n\t"
+#elif defined (_MIPS_ARCH_R5900)
+     ".set	mips3\n\t"
 #endif
      "sync\n\t"
      "1:\n\t"

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Fredrik Noring]

Hi Aleksandar,

> - Suggestion: The next MIPS pull request is scehuled for Friday,
> Jan 18, 2018. It would be fantastic if you could prepare the
> following by Jan 14:
> 
>   * Add 32 TCGv_i64 registers that would represent higher halves
>   of R5900 general purpose registers.

Done!

>   * Add TCGv_i32 register SA (shift amount).

See notes below.

>   * Perhaps consider adding higher halves of registers HI an LO
>   independently on HI/LO array used by DSP.

For HI1 and LO1 only? I'm asking since HI0 and LO0 are implemented with
the DSP array anyway, for all ISAs.

>   * It is customary to implement R/W access while introducing
>   such registers:
>     * Implement R/W access instructions to higher halves of
>     R5900 GPRs:
>       * LQ

Done, including PCPYUD and PCPYLD for proper testing!

>       * SQ

Done, including testing!

>     * Implement R/W access instructions to SA register:
>       * MFSA

The TX79 manual says that "the sole purpose of this instruction is to
permit the shift amount to be saved during a context switch" and that
"the shift amount is encoded in SA in an implementation-defined manner"
so it seems to make more sense for system mode rather than user mode?

One may want to choose an implementation that matches the actual R5900
hardware, even though the manual says it's arbitrary.

>       * MTSA

Likewise.

>       * MTSAH
>       * MTSAB

These instructions do not appear to be usable unless the corresponding
shift instructions are implemented as well?

I will post an R5900 multimedia instruction patch series shortly.

Fredrik

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Aleksandar Markovic]

On Sunday, January 13, 2019, Fredrik Noring <noring@nocrew.org> wrote:

> Hi Aleksandar,
>
> > - Suggestion: The next MIPS pull request is scehuled for Friday,
> > Jan 18, 2018. It would be fantastic if you could prepare the
> > following by Jan 14:
> >
> >   * Add 32 TCGv_i64 registers that would represent higher halves
> >   of R5900 general purpose registers.
>
> Done!


Awesome!

I am especially happy with your choice of naming "mmr" (MultiMedia
Registers) for these fieilds, since that is what they really are (and they
are certainly not "gprs"). Right on the money!


>
> >   * Add TCGv_i32 register SA (shift amount).
>
> See notes below.
>
> >   * Perhaps consider adding higher halves of registers HI an LO
> >   independently on HI/LO array used by DSP.
>
> For HI1 and LO1 only? I'm asking since HI0 and LO0 are implemented with
> the DSP array anyway, for all ISAs.
>
>
I leave it to your judgement. If you are not sure (or you find the current
implementation too sensitive or contrieved to touch), you can leave HI1/LO1
fields implementation as it is now. My motivation was avoiding usage of the
same data fields for two relatively independant purposes.


> >   * It is customary to implement R/W access while introducing
> >   such registers:
> >     * Implement R/W access instructions to higher halves of
> >     R5900 GPRs:
> >       * LQ
>
> Done, including PCPYUD and PCPYLD for proper testing!
>
>
Outstanding! I salute your including PCPYUD and PCPYLD in this group - they
too can be considered "basic R/W access to mmr".


> >       * SQ
>
> Done, including testing!
>
>
Perfect!


> >     * Implement R/W access instructions to SA register:
> >       * MFSA
>
> The TX79 manual says that "the sole purpose of this instruction is to
> permit the shift amount to be saved during a context switch" and that
> "the shift amount is encoded in SA in an implementation-defined manner"
> so it seems to make more sense for system mode rather than user mode?
>
> One may want to choose an implementation that matches the actual R5900
> hardware, even though the manual says it's arbitrary.
>
> >       * MTSA
>
> Likewise.
>
> >       * MTSAH
> >       * MTSAB
>
> These instructions do not appear to be usable unless the corresponding
> shift instructions are implemented as well?
>
>
The goal right now is to prepare basic stuff related to SA register, even
though there is possibly no immediate any application use case. However,
this will make potential future development considerably easier, so please
include handling of this register and these instructions.


> I will post an R5900 multimedia instruction patch series shortly.
>
>
Yes, go ahead, please!

I unfortunatelly will not have enough time for detailed review before
Tuesday, but I have a relatively simple hint right now:

Regarding segments:

+    int rs = extract32(ctx->opcode, 21, 5);
+    int rt = extract32(ctx->opcode, 16, 5);
+    int rd = extract32(ctx->opcode, 11, 5);

Please include them in gen_XXX() functions, rather than in decode_XXX()
functions. This will leave decode_XXX() functions with a single
responsibility of detecting what instruction is about to be processed,
which is cleaner from logical decomposition point of view (even if it would
sometimes result in the repetition of some code segments - logical
decomposition is of far greater importance).

Thanks for all efforts!!

Aleksandar




> Fredrik
>
>

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Fredrik Noring]

Hi Aleksandar,

> Awesome!
> 
> I am especially happy with your choice of naming "mmr" (MultiMedia
> Registers) for these fieilds, since that is what they really are (and they
> are certainly not "gprs"). Right on the money!

Great, thanks!

> > For HI1 and LO1 only? I'm asking since HI0 and LO0 are implemented with
> > the DSP array anyway, for all ISAs.
> 
> I leave it to your judgement. If you are not sure (or you find the current
> implementation too sensitive or contrieved to touch), you can leave HI1/LO1
> fields implementation as it is now. My motivation was avoiding usage of the
> same data fields for two relatively independant purposes.

I think the change is simple, but what should we call the new variables?

/* global register indices */
static TCGv cpu_gpr[32], cpu_PC;
static TCGv cpu_HI[MIPS_DSP_ACC], cpu_LO[MIPS_DSP_ACC];
static TCGv cpu_HI1, cpu_LO1;    /* Upper half of 128-bit TX79 HI and LO */

Something like the last line?

By the way, what are your thoughts on "[PATCH v2 3/6] target/mips: Fix
HI[ac] and LO[ac] 32-bit truncation with MIPS64 DSP ASE"?

https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg01287.html

> Outstanding! I salute your including PCPYUD and PCPYLD in this group - they
> too can be considered "basic R/W access to mmr".

Good, many thanks!

> The goal right now is to prepare basic stuff related to SA register, even
> though there is possibly no immediate any application use case. However,
> this will make potential future development considerably easier, so please
> include handling of this register and these instructions.

Done, although I have some minor clean-ups left to do. I have checked with
R5900 hardware to match the implementation defined value of the SA register.

I will post MFSA, MTSA, MTSAB and MTSAH in v2 of this patch series.

> Regarding segments:
> 
> +    int rs = extract32(ctx->opcode, 21, 5);
> +    int rt = extract32(ctx->opcode, 16, 5);
> +    int rd = extract32(ctx->opcode, 11, 5);
> 
> Please include them in gen_XXX() functions, rather than in decode_XXX()
> functions. This will leave decode_XXX() functions with a single
> responsibility of detecting what instruction is about to be processed,
> which is cleaner from logical decomposition point of view (even if it would
> sometimes result in the repetition of some code segments - logical
> decomposition is of far greater importance).

Done!

Fredrik

Re: [Qemu-devel] [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

[Aleksandar Markovic]

On Monday, January 14, 2019, Fredrik Noring <noring@nocrew.org> wrote:

> Hi Aleksandar,
>
> > Awesome!
> >
> > I am especially happy with your choice of naming "mmr" (MultiMedia
> > Registers) for these fieilds, since that is what they really are (and
> they
> > are certainly not "gprs"). Right on the money!
>
> Great, thanks!
>
> > > For HI1 and LO1 only? I'm asking since HI0 and LO0 are implemented with
> > > the DSP array anyway, for all ISAs.
> >
> > I leave it to your judgement. If you are not sure (or you find the
> current
> > implementation too sensitive or contrieved to touch), you can leave
> HI1/LO1
> > fields implementation as it is now. My motivation was avoiding usage of
> the
> > same data fields for two relatively independant purposes.
>
> I think the change is simple, but what should we call the new variables?
>
> /* global register indices */
> static TCGv cpu_gpr[32], cpu_PC;
> static TCGv cpu_HI[MIPS_DSP_ACC], cpu_LO[MIPS_DSP_ACC];
> static TCGv cpu_HI1, cpu_LO1;    /* Upper half of 128-bit TX79 HI and LO */
>
> Something like the last line?
>
>
Correct.


> By the way, what are your thoughts on "[PATCH v2 3/6] target/mips: Fix
> HI[ac] and LO[ac] 32-bit truncation with MIPS64 DSP ASE"?
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg01287.html
>
>
Still taking a closer look. Didn't forget.


> > Outstanding! I salute your including PCPYUD and PCPYLD in this group -
> they
> > too can be considered "basic R/W access to mmr".
>
> Good, many thanks!
>
> > The goal right now is to prepare basic stuff related to SA register, even
> > though there is possibly no immediate any application use case. However,
> > this will make potential future development considerably easier, so
> please
> > include handling of this register and these instructions.
>
> Done, although I have some minor clean-ups left to do. I have checked with
> R5900 hardware to match the implementation defined value of the SA
> register.
>
> I will post MFSA, MTSA, MTSAB and MTSAH in v2 of this patch series.
>
>
Magnificent!


> > Regarding segments:
> >
> > +    int rs = extract32(ctx->opcode, 21, 5);
> > +    int rt = extract32(ctx->opcode, 16, 5);
> > +    int rd = extract32(ctx->opcode, 11, 5);
> >
> > Please include them in gen_XXX() functions, rather than in decode_XXX()
> > functions. This will leave decode_XXX() functions with a single
> > responsibility of detecting what instruction is about to be processed,
> > which is cleaner from logical decomposition point of view (even if it
> would
> > sometimes result in the repetition of some code segments - logical
> > decomposition is of far greater importance).
>
> Done!
>
> Fredrik
>

[Qemu-devel] [PATCH v2 10/12] disas/mips: Increase 'member of ISAs' flag holder size

[Fredrik Noring]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Increase the size of 'membership' holder size to 64 bits. This is
needed for future extensions since existing bits are almost all used.
This change is related to commit f9c9cd63e3 "target/mips: Increase
'supported ISAs/ASEs' flag holder size".

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Fredrik Noring <noring@nocrew.org>
Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 disas/mips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/mips.c b/disas/mips.c
index 97f661a37e..d73d4094d8 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -301,7 +301,7 @@ struct mips_opcode
   unsigned long pinfo2;
   /* A collection of bits describing the instruction sets of which this
      instruction or macro is a member. */
-  unsigned long membership;
+  uint64_t membership;
 };
 
 /* These are the characters which may appear in the args field of an
-- 
2.18.1

[Qemu-devel] [PATCH v2 11/12] disas/mips: Define R5900 disassembly constants

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 disas/mips.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/disas/mips.c b/disas/mips.c
index d73d4094d8..9f01fda8bd 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -611,6 +611,9 @@ struct mips_opcode
 /* ST Microelectronics Loongson 2F.  */
 #define INSN_LOONGSON_2F          0x80000000
 
+/* Sony/Toshiba R5900 */
+#define INSN_5900                 0x100000000
+
 /* MIPS ISA defines, use instead of hardcoding ISA level.  */
 
 #define       ISA_UNKNOWN     0               /* Gas internal use.  */
@@ -646,6 +649,7 @@ struct mips_opcode
 #define CPU_R5000	5000
 #define CPU_VR5400	5400
 #define CPU_VR5500	5500
+#define CPU_R5900       5900
 #define CPU_R6000	6000
 #define CPU_RM7000	7000
 #define CPU_R8000	8000
@@ -1193,6 +1197,7 @@ extern const int bfd_mips16_num_opcodes;
 #define N5	(INSN_5400 | INSN_5500)
 #define N54	INSN_5400
 #define N55	INSN_5500
+#define EE      INSN_5900    /* Emotion Engine */
 
 #define G1      (T3             \
                  )
@@ -3861,6 +3866,7 @@ struct mips_arch_choice
 #define bfd_mach_mips5000              5000
 #define bfd_mach_mips5400              5400
 #define bfd_mach_mips5500              5500
+#define bfd_mach_mips5900              5900
 #define bfd_mach_mips6000              6000
 #define bfd_mach_mips7000              7000
 #define bfd_mach_mips8000              8000
@@ -3908,6 +3914,8 @@ static const struct mips_arch_choice mips_arch_choices[] =
     mips_cp0_names_numeric, NULL, 0, mips_hwr_names_numeric },
   { "vr5500",	1, bfd_mach_mips5500, CPU_VR5500, ISA_MIPS4,
     mips_cp0_names_numeric, NULL, 0, mips_hwr_names_numeric },
+  { "r5900",    1, bfd_mach_mips5900, CPU_R5900, ISA_MIPS3,
+    mips_cp0_names_numeric, NULL, 0, mips_hwr_names_numeric },
   { "r6000",	1, bfd_mach_mips6000, CPU_R6000, ISA_MIPS2,
     mips_cp0_names_numeric, NULL, 0, mips_hwr_names_numeric },
   { "rm7000",	1, bfd_mach_mips7000, CPU_RM7000, ISA_MIPS4,
-- 
2.18.1

[Qemu-devel] [PATCH v2 12/12] disas/mips: Disassemble R5900 DIV[U]1, M{F, T}{LO, HI}1 and MULT[U]1

[Fredrik Noring]

Disassemble the R5900 instructions DIV1, DIVU1, MFLO1, MTLO1, MFHI1,
MTHI1, MULT1 and MULTU1. The opcodes for MADD1 and MADDU1 clash with
the opcodes for CLZ and CLO, resulting in incorrect disassembly. They
are therefore omitted here.

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 disas/mips.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/disas/mips.c b/disas/mips.c
index 9f01fda8bd..eddfb59325 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -2323,6 +2323,8 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"div",     "z,t",      0x0000001a, 0xffe0ffff, RD_s|RD_t|WR_HILO,      0,		I1      },
 {"div",     "d,v,t",	0,    (int) M_DIV_3,	INSN_MACRO,		0,		I1	},
 {"div",     "d,v,I",	0,    (int) M_DIV_3I,	INSN_MACRO,		0,		I1	},
+{"div1",    "z,s,t",  0x7000001a, 0xfc00ffff, RD_s | RD_t | WR_HILO, 0, EE },
+{"div1",    "z,t",    0x7000001a, 0xffe0ffff, RD_s | RD_t | WR_HILO, 0, EE },
 {"div.d",   "D,V,T",	0x46200003, 0xffe0003f,	WR_D|RD_S|RD_T|FP_D,	0,		I1	},
 {"div.s",   "D,V,T",	0x46000003, 0xffe0003f,	WR_D|RD_S|RD_T|FP_S,	0,		I1	},
 {"div.ps",  "D,V,T",	0x46c00003, 0xffe0003f,	WR_D|RD_S|RD_T|FP_D,	0,		SB1	},
@@ -2331,6 +2333,8 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"divu",    "z,t",      0x0000001b, 0xffe0ffff, RD_s|RD_t|WR_HILO,      0,		I1      },
 {"divu",    "d,v,t",	0,    (int) M_DIVU_3,	INSN_MACRO,		0,		I1	},
 {"divu",    "d,v,I",	0,    (int) M_DIVU_3I,	INSN_MACRO,		0,		I1	},
+{"divu1",   "z,s,t",  0x7000001b, 0xfc00ffff, RD_s | RD_t | WR_HILO, 0, EE },
+{"divu1",   "z,t",    0x7000001b, 0xffe0ffff, RD_s | WR_HILO       , 0, EE },
 {"dla",     "t,A(b)",	0,    (int) M_DLA_AB,	INSN_MACRO,		0,		I3	},
 {"dlca",    "t,A(b)",	0,    (int) M_DLCA_AB,	INSN_MACRO,		0,		I3	},
 {"dli",     "t,j",      0x24000000, 0xffe00000, WR_t,			0,		I3	}, /* addiu */
@@ -2594,8 +2598,10 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"mfdr",    "t,G",	0x7000003d, 0xffe007ff,	LCD|WR_t|RD_C0,		0,		N5      },
 {"mfhi",    "d",	0x00000010, 0xffff07ff,	WR_d|RD_HI,		0,		I1	},
 {"mfhi",    "d,9",	0x00000010, 0xff9f07ff, WR_d|RD_HI,		0,		D32	},
+{"mfhi1",   "d",  0x70000010, 0xffff07ff, WR_d | RD_HI, 0, EE},
 {"mflo",    "d",	0x00000012, 0xffff07ff,	WR_d|RD_LO,		0,		I1	},
 {"mflo",    "d,9",	0x00000012, 0xff9f07ff, WR_d|RD_LO,		0,		D32	},
+{"mflo1",   "d",  0x70000012, 0xffff07ff, WR_d | RD_LO, 0, EE},
 {"mflhxu",  "d",	0x00000052, 0xffff07ff,	WR_d|MOD_HILO,		0,		SMT	},
 {"min.ob",  "X,Y,Q",	0x78000006, 0xfc20003f,	WR_D|RD_S|RD_T|FP_D,	0,		MX|SB1	},
 {"min.ob",  "D,S,T",	0x4ac00006, 0xffe0003f,	WR_D|RD_S|RD_T,		0,		N54	},
@@ -2661,8 +2667,10 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"mtdr",    "t,G",	0x7080003d, 0xffe007ff,	COD|RD_t|WR_C0,		0,		N5	},
 {"mthi",    "s",	0x00000011, 0xfc1fffff,	RD_s|WR_HI,		0,		I1	},
 {"mthi",    "s,7",	0x00000011, 0xfc1fe7ff, RD_s|WR_HI,		0,		D32	},
+{"mthi1",   "s",  0x70000011, 0xfc1fffff, RD_s | WR_HI, 0, EE },
 {"mtlo",    "s",	0x00000013, 0xfc1fffff,	RD_s|WR_LO,		0,		I1	},
 {"mtlo",    "s,7",	0x00000013, 0xfc1fe7ff, RD_s|WR_LO,		0,		D32	},
+{"mtlo1",   "s",  0x70000013, 0xfc1fffff, RD_s | WR_LO, 0, EE },
 {"mtlhx",   "s",	0x00000053, 0xfc1fffff,	RD_s|MOD_HILO,		0,		SMT	},
 {"mttc0",   "t,G",	0x41800000, 0xffe007ff, TRAP|COD|RD_t|WR_C0|WR_CC, 0,		MT32	},
 {"mttc0",   "t,+D",	0x41800000, 0xffe007f8, TRAP|COD|RD_t|WR_C0|WR_CC, 0,		MT32	},
@@ -2728,10 +2736,14 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"mult",    "s,t",      0x00000018, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0,		I1	},
 {"mult",    "7,s,t",	0x00000018, 0xfc00e7ff, WR_a|RD_s|RD_t,         0,              D33	},
 {"mult",    "d,s,t",    0x00000018, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, 0,		G1	},
+{"mult1",   "s,t",      0x70000018, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE },
+{"mult1",   "d,s,t", 0x70000018, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, EE },
 {"multp",   "s,t",	0x00000459, 0xfc00ffff,	RD_s|RD_t|MOD_HILO,	0,		SMT	},
 {"multu",   "s,t",      0x00000019, 0xfc00ffff, RD_s|RD_t|WR_HILO|IS_M, 0,		I1	},
 {"multu",   "7,s,t",	0x00000019, 0xfc00e7ff, WR_a|RD_s|RD_t,         0,              D33	},
 {"multu",   "d,s,t",    0x00000019, 0xfc0007ff, RD_s|RD_t|WR_HILO|WR_d|IS_M, 0,		G1	},
+{"multu1",  "s,t",      0x70000019, 0xfc00ffff, RD_s | RD_t | WR_HILO | IS_M, 0, EE },
+{"multu1",  "d,s,t", 0x70000019, 0xfc0007ff, WR_d | RD_s | RD_t | WR_HILO | IS_M, 0, EE },
 {"mulu",    "d,s,t",	0x00000059, 0xfc0007ff,	RD_s|RD_t|WR_HILO|WR_d,	0,		N5	},
 {"neg",     "d,w",	0x00000022, 0xffe007ff,	WR_d|RD_t,		0,		I1	}, /* sub 0 */
 {"negu",    "d,w",	0x00000023, 0xffe007ff,	WR_d|RD_t,		0,		I1	}, /* subu 0 */
-- 
2.18.1

Re: [Qemu-devel] [PATCH v2 12/12] disas/mips: Disassemble R5900 DIV[U]1, M{F, T}{LO, HI}1 and MULT[U]1

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
>
> Subject: [PATCH v2 12/12] disas/mips: Disassemble R5900 DIV[U]1, M{F,T}{LO,HI}1 and MULT[U]1
>
> Disassemble the R5900 instructions DIV1, DIVU1, MFLO1, MTLO1, MFHI1,
> MTHI1, MULT1 and MULTU1. The opcodes for MADD1 and MADDU1 clash with
> the opcodes for CLZ and CLO, resulting in incorrect disassembly. They
> are therefore omitted here.
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---

Hi, Fredrik,

I am glad that you want to include QEMU disas support for R5900 - this area usually gets forgotten.

But, as you can see, this MIPS feature is partially broken - it doesn't handle well overlapping opcodes, and the field "membership" is not taken into account at all. I think the feature should be fixed first, and then R5900 support added. In fact, the disassembler support in QEMU is almost independent on the emulation support (for the corresponding instructions) - so, we could add disassembler support for all R5900 instructions in one clean sweep, instead dividing that in "million" pieces.

The key to the successful solution would be detecting what CPU is currently being emulated, and making disassembling decision based on that.

Let's talk about that later.

Sincerely,
Aleksandar

P.S. Sorry for misspelling your name on several occasions.

Re: [Qemu-devel] [PATCH v2 12/12] disas/mips: Disassemble R5900 DIV[U]1, M{F, T}{LO, HI}1 and MULT[U]1

[Fredrik Noring]

Hi Aleksandar,

> I am glad that you want to include QEMU disas support for R5900 - this
> area usually gets forgotten.
> 
> But, as you can see, this MIPS feature is partially broken - it doesn't
> handle well overlapping opcodes, and the field "membership" is not taken
> into account at all. I think the feature should be fixed first, and then
> R5900 support added. In fact, the disassembler support in QEMU is almost
> independent on the emulation support (for the corresponding instructions)
> - so, we could add disassembler support for all R5900 instructions in one
> clean sweep, instead dividing that in "million" pieces.
> 
> The key to the successful solution would be detecting what CPU is
> currently being emulated, and making disassembling decision based on that.
> 
> Let's talk about that later.

Yes, the current disassembly table needs to be reworked, so let's postpone
the opcodes for the R5900.

> P.S. Sorry for misspelling your name on several occasions.

No problem!

Fredrik