Skip to contentSkip to conversation

target/mips: Amend R5900 support

PATCH In review Fredrik Noring 3 participants 28 messages Nov 1, 2018 – Jan 15, 2019

lore
Share & tools
lore

Fredrik Noring

author
#0 lore raw

v2 cover target/mips: Amend R5900 support

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
Signature · 1 line
2.18.1
author
#1 lore raw

PATCH v2 1/12 target/mips: Generate R5900 MFLO1, MFHI1, MTLO1 and MTHI1 in gen_HILO1_tx79

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

Signed-off-by Fredrik Noring <[email protected]>
---
 target/mips/translate.c | 67 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 11 deletions(-)
target/mips/translate.c +56 -11
index 60320cbe69..f3993cf7d7 100644
@@ -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 */
Signature · 1 line
2.18.1
author
#2 lore raw

PATCH v2 2/12 target/mips: Generate R5900 DIV1 and DIVU1 in gen_div1_tx79

In reply to Fredrik Noring

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

Signed-off-by Fredrik Noring <[email protected]>
---
 target/mips/translate.c | 65 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)
target/mips/translate.c +59 -6
index f3993cf7d7..6e5a8a2565 100644
@@ -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:
Signature · 1 line
2.18.1
author
#3 lore raw

PATCH v2 3/12 target/mips: R5900 LQ and SQ also belong to the Toshiba MMI ASE

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 target/mips/translate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
target/mips/translate.c +4 -2
index 6e5a8a2565..624e53644d 100644
@@ -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. */
Signature · 1 line
2.18.1
author
#4 lore raw

PATCH v2 4/12 target/mips: Support Toshiba specific three-operand MADD and MADDU

In reply to Fredrik Noring

From: Philippe Mathieu-Daudé <[email protected]>

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é<[email protected]>
Signed-off-by Fredrik Noring <[email protected]>
Tested-by Fredrik Noring <[email protected]>
---
 target/mips/translate.c | 58 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 5 deletions(-)
target/mips/translate.c +53 -5
index 624e53644d..4808cb49c3 100644
@@ -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 */
Signature · 1 line
2.18.1
author
#6 lore raw

PATCH v2 5/12 target/mips: Support R5900 three-operand MADD1 and MADDU1

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 target/mips/translate.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
target/mips/translate.c +9 -3
index 4808cb49c3..57b17ad8f6 100644
@@ -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 */
Signature · 1 line
2.18.1
author
#8 lore raw

PATCH v2 6/12 tests/tcg/mips: Test R5900 three-operand MADD

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 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
tests/tcg/mips/mipsr5900/Makefile +1 -0
index a1c388bc3c..97ca2a671c 100644
@@ -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
tests/tcg/mips/mipsr5900/madd.c new file +45 -0

45 lines not on this page · raw message

Signature · 1 line
2.18.1
author
#10 lore raw

PATCH v2 7/12 tests/tcg/mips: Test R5900 three-operand MADD1

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 tests/tcg/mips/mipsr5900/madd.c | 43 +++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)
tests/tcg/mips/mipsr5900/madd.c +38 -5
index 9ad2ea6dbb..f6f215e1c3 100644
@@ -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()
Signature · 1 line
2.18.1
author
#12 lore raw

PATCH v2 8/12 tests/tcg/mips: Test R5900 three-operand MADDU

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 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
tests/tcg/mips/mipsr5900/Makefile +1 -0
index 97ca2a671c..27ee5d5f54 100644
@@ -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
tests/tcg/mips/mipsr5900/maddu.c new file +37 -0
new file mode 100644
index 0000000000..e4e552102d
@@ -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;
+}
Signature · 1 line
2.18.1
author
#14 lore raw

PATCH v2 9/12 tests/tcg/mips: Test R5900 three-operand MADDU1

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 tests/tcg/mips/mipsr5900/maddu.c | 37 ++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)
tests/tcg/mips/mipsr5900/maddu.c +35 -2
index e4e552102d..30936fb2b4 100644
@@ -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;
 }
Signature · 1 line
2.18.1
author
#23 lore raw

PATCH v2 10/12 disas/mips: Increase 'member of ISAs' flag holder size

In reply to Fredrik Noring

From: Philippe Mathieu-Daudé <[email protected]>

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é <[email protected]>
Reviewed-by Richard Henderson <[email protected]>
Tested-by Fredrik Noring <[email protected]>
Signed-off-by Fredrik Noring <[email protected]>
---
 disas/mips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
disas/mips.c +1 -1
index 97f661a37e..d73d4094d8 100644
@@ -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
Signature · 1 line
2.18.1
author
#24 lore raw

PATCH v2 11/12 disas/mips: Define R5900 disassembly constants

In reply to Fredrik Noring
Signed-off-by Fredrik Noring <[email protected]>
---
 disas/mips.c | 8 ++++++++
 1 file changed, 8 insertions(+)
disas/mips.c +8 -0
index d73d4094d8..9f01fda8bd 100644
@@ -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,
Signature · 1 line
2.18.1
author
#25 lore raw

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

In reply to 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 <[email protected]>
---
 disas/mips.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
disas/mips.c +12 -0
index 9f01fda8bd..eddfb59325 100644
@@ -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 */
Signature · 1 line
2.18.1
#26 lore raw

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

Quoted 11 lines

From: Fredrik Noring <[email protected]>

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 <[email protected]>

---

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.

author
#27 lore raw

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

Hi Aleksandar,

Quoted 11 lines

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.

Quoted 4 lines

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.

Quoted 1 line

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

No problem!

Fredrik

Reviewed-by on v2 4/12
#5 lore raw

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

In reply to Fredrik Noring
Quoted 4 lines

From: Fredrik Noring <[email protected]>
Subject: [PATCH v2 04/12] target/mips: Support Toshiba specific three-operand MADD and MADDU

From: Philippe Mathieu-Daudé <[email protected]>

Reviewed-by Aleksandar Markovic <[email protected]>

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

author
#16 lore raw

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

Thanks Aleksandar!

Quoted 8 lines
Quoted 2 lines

From: Fredrik Noring <[email protected]>
Subject: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

Reviewed-by Aleksandar Markovic <[email protected]>

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

#17 lore raw

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

Quoted 14 lines

From: Fredrik Noring <[email protected]>
Sent: Tuesday, January 1, 2019 7:27 PM
To: Aleksandar Markovic
Cc: Aurelien Jarno; Philippe Mathieu-Daudé; Jürgen Urban; Maciej W. Rozycki; [email protected]
Subject: Re: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

Thanks Aleksandar!

Quoted 5 lines
Quoted 2 lines

From: Fredrik Noring <[email protected]>
Subject: [PATCH v2 09/12] tests/tcg/mips: Test R5900 three-operand MADDU1

Reviewed-by Aleksandar Markovic <[email protected]>
Quoted 8 lines
Quoted 3 lines

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

author
#18 lore raw

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

Hi Aleksandar,

Quoted 1 line

Glad to see you back!

Likewise!

Quoted 1 line

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

Great!

Quoted 4 lines

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.

Quoted 2 lines

(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:

linux-user/mips64/target_elf.h +3 -0
@@ -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
Quoted 4 lines

- 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:

Quoted 16 lines

* 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!

Quoted 4 lines

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.

Quoted 2 lines

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

Thanks!

Quoted 4 lines

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

sysdeps/mips/mips64/addmul_1.S +3 -0
@@ -33,6 +33,9 @@
 	.option pic2
 #endif
 ENTRY (__mpn_addmul_1)
+#ifdef _MIPS_ARCH_R5900
+	.set	mips3
+#endif
 #ifdef PIC
 	SETUP_GP /* ??? unused */
 #endif
sysdeps/mips/mips64/mul_1.S +3 -0
index 8707257a68..19e3ae5eae 100644
@@ -34,6 +34,9 @@
 	.option pic2
 #endif
 ENTRY (__mpn_mul_1)
+#ifdef _MIPS_ARCH_R5900
+	.set	mips3
+#endif
 #ifdef __PIC__
 	SETUP_GP /* ??? unused */
 #endif
sysdeps/mips/mips64/submul_1.S +3 -0
index fb9a1c2375..763c5bb687 100644
@@ -34,6 +34,9 @@
 	.option pic2
 #endif
 ENTRY (__mpn_submul_1)
+#ifdef _MIPS_ARCH_R5900
+	.set	mips3
+#endif
 #ifdef __PIC__
 	SETUP_GP /* ??? unused */
 #endif
sysdeps/mips/sys/tas.h +3 -0
index d5ed013e28..ad797bfb1d 100644
@@ -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"
author
#19 lore raw

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

In reply to Aleksandar Markovic

Hi Aleksandar,

Quoted 6 lines

- 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!

Quoted 1 line

* Add TCGv_i32 register SA (shift amount).

See notes below.

Quoted 2 lines

* 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.

Quoted 5 lines

* 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!

Quoted 1 line

* SQ

Done, including testing!

Quoted 2 lines

* 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.

Quoted 1 line

* MTSA

Likewise.

Quoted 2 lines

* 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

#20 lore raw

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

Quoted 10 lines · Fredrik Noring

Hi Aleksandar,

Quoted 6 lines

- 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!

Quoted 12 lines
Quoted 2 lines

* Add TCGv_i32 register SA (shift amount).

See notes below.

Quoted 2 lines

* 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.

Quoted 9 lines
Quoted 5 lines

* 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".

Quoted 5 lines
Quoted 1 line

* SQ

Done, including testing!

Perfect!

Quoted 13 lines
Quoted 2 lines

* 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.

Quoted 1 line

* MTSA

Quoted 9 lines

Likewise.

Quoted 2 lines

* 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.

Quoted 3 lines

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

Quoted 3 lines

Fredrik

author
#21 lore raw

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

Hi Aleksandar,

Quoted 5 lines

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!

Quoted 7 lines
Quoted 2 lines

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

Quoted 2 lines

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!

Quoted 4 lines

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.

Quoted 6 lines

Regarding segments:

(quoted diff) +3 -0
+    int rs = extract32(ctx->opcode, 21, 5);
+    int rt = extract32(ctx->opcode, 16, 5);
+    int rd = extract32(ctx->opcode, 11, 5);
Quoted 6 lines

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

#22 lore raw

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

Quoted 22 lines · Fredrik Noring

Hi Aleksandar,

Quoted 4 lines

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

Quoted 1 line

are certainly not "gprs"). Right on the money!

Great, thanks!

Quoted 4 lines
Quoted 2 lines

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

Quoted 1 line

implementation too sensitive or contrieved to touch), you can leave

HI1/LO1

Quoted 1 line

fields implementation as it is now. My motivation was avoiding usage of

the

Quoted 1 line

same data fields for two relatively independant purposes.

Quoted 10 lines

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.

Quoted 6 lines

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.

Quoted 9 lines
Quoted 1 line

Outstanding! I salute your including PCPYUD and PCPYLD in this group -

they

Quoted 1 line

too can be considered "basic R/W access to mmr".

Good, many thanks!

Quoted 3 lines

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

Quoted 10 lines

please

Quoted 1 line

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!

Quoted 18 lines
Quoted 10 lines

Regarding segments:

(quoted diff) +3 -0
+    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

Quoted 2 lines

sometimes result in the repetition of some code segments - logical decomposition is of far greater importance).

Done!

Fredrik