[Qemu-devel] [PATCH 0/7] Add limited MXU instruction support

[Qemu-devel] [PATCH 0/7] Add limited MXU instruction support

[Craig Janeczek]

This patch set begins to add MXU instruction support for mips
emulation. The patches are split such that the register overhead
is added first followed by a series of instructions.

Craig Janeczek (7):
  target/mips: Add MXU register support
  target/mips: Add MXU instructions S32I2M and S32M2I
  target/mips: Add MXU instruction S8LDD
  target/mips: Add MXU instruction D16MUL
  target/mips: Add MXU instruction D16MAC
  target/mips: Add MXU instructions Q8MUL and Q8MULSU
  target/mips: Add MXU instructions S32LDD and S32LDDR

 target/mips/cpu.h       |   1 +
 target/mips/translate.c | 401 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 398 insertions(+), 4 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH 1/7] target/mips: Add MXU register support

[Craig Janeczek]

This commit makes the MXU registers and the helper functions for
reading/writing to them. This is required for full MXU instruction
support.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/cpu.h       |  1 +
 target/mips/translate.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 009202cf64..4b2948a2c8 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -170,6 +170,7 @@ struct TCState {
         MSACSR_FS_MASK)
 
     float_status msa_fp_status;
+    target_ulong mxu_gpr[16];
 };
 
 typedef struct CPUMIPSState CPUMIPSState;
diff --git a/target/mips/translate.c b/target/mips/translate.c
index bdd880bb77..50f0cb558f 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1398,6 +1398,9 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
 
+/* MXU registers */
+static TCGv mxu_gpr[16];
+
 #include "exec/gen-icount.h"
 
 #define gen_helper_0e0i(name, arg) do {                           \
@@ -1517,6 +1520,13 @@ static const char * const msaregnames[] = {
     "w30.d0", "w30.d1", "w31.d0", "w31.d1",
 };
 
+static const char * const mxuregnames[] = {
+    "XR1",  "XR2",  "XR3",  "XR4",  "XR5",
+    "XR6",  "XR7",  "XR8",  "XR9",  "XR10",
+    "XR11", "XR12", "XR13", "XR14", "XR15",
+    "XR16",
+};
+
 #define LOG_DISAS(...)                                                        \
     do {                                                                      \
         if (MIPS_DEBUG_DISAS) {                                               \
@@ -1550,6 +1560,21 @@ static inline void gen_store_gpr (TCGv t, int reg)
         tcg_gen_mov_tl(cpu_gpr[reg], t);
 }
 
+/* MXU General purpose registers moves. */
+static inline void gen_load_mxu_gpr (TCGv t, int reg)
+{
+    if (reg == 0)
+        tcg_gen_movi_tl(t, 0);
+    else
+        tcg_gen_mov_tl(t, mxu_gpr[reg-1]);
+}
+
+static inline void gen_store_mxu_gpr (TCGv t, int reg)
+{
+    if (reg != 0)
+        tcg_gen_mov_tl(mxu_gpr[reg-1], t);
+}
+
 /* Moves to/from shadow registers. */
 static inline void gen_load_srsgpr (int from, int to)
 {
@@ -20742,6 +20767,11 @@ void mips_tcg_init(void)
     fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
+
+    for (i = 0; i < 16; i++)
+        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
+                                        offsetof(CPUMIPSState, active_tc.mxu_gpr[i]),
+                                        mxuregnames[i]);
 }
 
 #include "translate_init.inc.c"
-- 
2.18.0

[Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

[Craig Janeczek]

Adds support for emulating the S32I2M and S32M2I MXU instructions.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/translate.c | 55 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 50f0cb558f..381dfad36e 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -364,6 +364,9 @@ enum {
     OPC_CLO      = 0x21 | OPC_SPECIAL2,
     OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
     OPC_DCLO     = 0x25 | OPC_SPECIAL2,
+    /* MXU */
+    OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
+    OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,
     /* Special */
     OPC_SDBBP    = 0x3F | OPC_SPECIAL2,
 };
@@ -3763,6 +3766,52 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
     }
 }
 
+typedef union {
+    struct {
+        uint32_t op:6;
+        uint32_t xra:5;
+        uint32_t:5;
+        uint32_t rb:5;
+        uint32_t:5;
+        uint32_t special2:6;
+    } S32I2M;
+
+    struct {
+        uint32_t op:6;
+        uint32_t xra:5;
+        uint32_t:5;
+        uint32_t rb:5;
+        uint32_t:5;
+        uint32_t special2:6;
+    } S32M2I;
+} MXU_OPCODE;
+
+/* MXU Instructions */
+static void gen_mxu(DisasContext *ctx, uint32_t opc)
+{
+#ifndef TARGET_MIPS64 /* Only works in 32 bit mode */
+    TCGv t0;
+    t0 = tcg_temp_new();
+    MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
+
+    switch (opc) {
+    case OPC_MXU_S32I2M:
+        gen_load_gpr(t0, opcode->S32I2M.rb);
+        gen_store_mxu_gpr(t0, opcode->S32I2M.xra);
+        break;
+
+    case OPC_MXU_S32M2I:
+        gen_load_mxu_gpr(t0, opcode->S32M2I.xra);
+        gen_store_gpr(t0, opcode->S32M2I.rb);
+        break;
+    }
+
+    tcg_temp_free(t0);
+#else
+    generate_exception_end(ctx, EXCP_RI);
+#endif
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -17843,6 +17892,12 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         check_insn(ctx, INSN_LOONGSON2F);
         gen_loongson_integer(ctx, op1, rd, rs, rt);
         break;
+
+    case OPC_MXU_S32I2M:
+    case OPC_MXU_S32M2I:
+        gen_mxu(ctx, op1);
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

[Qemu-devel] [PATCH 3/7] target/mips: Add MXU instruction S8LDD

[Craig Janeczek]

Adds support for emulating the S8LDD MXU instruction.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/translate.c | 82 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 381dfad36e..4ccccd5849 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -365,6 +365,7 @@ enum {
     OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
     OPC_DCLO     = 0x25 | OPC_SPECIAL2,
     /* MXU */
+    OPC_MXU_S8LDD  = 0x22 | OPC_SPECIAL2,
     OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
     OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,
     /* Special */
@@ -3784,14 +3785,24 @@ typedef union {
         uint32_t:5;
         uint32_t special2:6;
     } S32M2I;
+
+    struct {
+        uint32_t op:6;
+        uint32_t xra:4;
+        uint32_t s8:8;
+        uint32_t optn3:3;
+        uint32_t rb:5;
+        uint32_t special2:6;
+    } S8LDD;
 } MXU_OPCODE;
 
 /* MXU Instructions */
 static void gen_mxu(DisasContext *ctx, uint32_t opc)
 {
 #ifndef TARGET_MIPS64 /* Only works in 32 bit mode */
-    TCGv t0;
+    TCGv t0, t1;
     t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
     MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
 
     switch (opc) {
@@ -3804,9 +3815,77 @@ static void gen_mxu(DisasContext *ctx, uint32_t opc)
         gen_load_mxu_gpr(t0, opcode->S32M2I.xra);
         gen_store_gpr(t0, opcode->S32M2I.rb);
         break;
+
+    case OPC_MXU_S8LDD:
+        gen_load_gpr(t0, opcode->S8LDD.rb);
+        tcg_gen_movi_tl(t1, opcode->S8LDD.s8);
+        tcg_gen_ext8s_tl(t1, t1);
+        tcg_gen_add_tl(t0, t0, t1);
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_SB);
+        switch (opcode->S8LDD.optn3) {
+        case 0: /*XRa[7:0] = tmp8 */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            gen_load_mxu_gpr(t0, opcode->S8LDD.xra);
+            tcg_gen_andi_tl(t0, t0, 0xFFFFFF00);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 1: /* XRa[15:8] = tmp8 */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            gen_load_mxu_gpr(t0, opcode->S8LDD.xra);
+            tcg_gen_andi_tl(t0, t0, 0xFFFF00FF);
+            tcg_gen_shli_tl(t1, t1, 8);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 2: /* XRa[23:16] = tmp8 */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            gen_load_mxu_gpr(t0, opcode->S8LDD.xra);
+            tcg_gen_andi_tl(t0, t0, 0xFF00FFFF);
+            tcg_gen_shli_tl(t1, t1, 16);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 3: /* XRa[31:24] = tmp8 */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            gen_load_mxu_gpr(t0, opcode->S8LDD.xra);
+            tcg_gen_andi_tl(t0, t0, 0x00FFFFFF);
+            tcg_gen_shli_tl(t1, t1, 24);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 4: /* XRa = {8'b0, tmp8, 8'b0, tmp8} */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            tcg_gen_mov_tl(t0, t1);
+            tcg_gen_shli_tl(t1, t1, 16);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 5: /* XRa = {tmp8, 8'b0, tmp8, 8'b0} */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            tcg_gen_shli_tl(t1, t1, 8);
+            tcg_gen_mov_tl(t0, t1);
+            tcg_gen_shli_tl(t1, t1, 16);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 6: /* XRa = {{8{sign of tmp8}}, tmp8, {8{sign of tmp8}}, tmp8} */
+            tcg_gen_mov_tl(t0, t1);
+            tcg_gen_andi_tl(t0, t0, 0xFF00FFFF);
+            tcg_gen_shli_tl(t1, t1, 16);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        case 7: /* XRa = {tmp8, tmp8, tmp8, tmp8} */
+            tcg_gen_andi_tl(t1, t1, 0xFF);
+            tcg_gen_mov_tl(t0, t1);
+            tcg_gen_shli_tl(t1, t1, 8);
+            tcg_gen_or_tl(t0, t0, t1);
+            tcg_gen_shli_tl(t1, t1, 8);
+            tcg_gen_or_tl(t0, t0, t1);
+            tcg_gen_shli_tl(t1, t1, 8);
+            tcg_gen_or_tl(t0, t0, t1);
+            break;
+        }
+        gen_store_mxu_gpr(t0, opcode->S8LDD.xra);
+        break;
     }
 
     tcg_temp_free(t0);
+    tcg_temp_free(t1);
 #else
     generate_exception_end(ctx, EXCP_RI);
 #endif
@@ -17895,6 +17974,7 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
 
     case OPC_MXU_S32I2M:
     case OPC_MXU_S32M2I:
+    case OPC_MXU_S8LDD:
         gen_mxu(ctx, op1);
         break;
 
-- 
2.18.0

[Qemu-devel] [PATCH 4/7] target/mips: Add MXU instruction D16MUL

[Craig Janeczek]

Adds support for emulating the D16MUL instruction.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/translate.c | 55 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 4ccccd5849..64fc6089bb 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -365,6 +365,7 @@ enum {
     OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
     OPC_DCLO     = 0x25 | OPC_SPECIAL2,
     /* MXU */
+    OPC_MXU_D16MUL = 0x08 | OPC_SPECIAL2,
     OPC_MXU_S8LDD  = 0x22 | OPC_SPECIAL2,
     OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
     OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,
@@ -3794,15 +3795,28 @@ typedef union {
         uint32_t rb:5;
         uint32_t special2:6;
     } S8LDD;
+
+    struct {
+        uint32_t op:6;
+        uint32_t xra:4;
+        uint32_t xrb:4;
+        uint32_t xrc:4;
+        uint32_t xrd:4;
+        uint32_t optn2:2;
+        uint32_t sel:2;
+        uint32_t special2:6;
+    } D16MUL;
 } MXU_OPCODE;
 
 /* MXU Instructions */
 static void gen_mxu(DisasContext *ctx, uint32_t opc)
 {
 #ifndef TARGET_MIPS64 /* Only works in 32 bit mode */
-    TCGv t0, t1;
+    TCGv t0, t1, t2, t3;
     t0 = tcg_temp_new();
     t1 = tcg_temp_new();
+    t2 = tcg_temp_new();
+    t3 = tcg_temp_new();
     MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
 
     switch (opc) {
@@ -3882,10 +3896,48 @@ static void gen_mxu(DisasContext *ctx, uint32_t opc)
         }
         gen_store_mxu_gpr(t0, opcode->S8LDD.xra);
         break;
+
+    case OPC_MXU_D16MUL:
+        if (opcode->D16MUL.sel == 1) {
+            /* D16MULE is not supported */
+            generate_exception_end(ctx, EXCP_RI);
+        }
+        gen_load_mxu_gpr(t1, opcode->D16MUL.xrb);
+        tcg_gen_ext16s_tl(t0, t1);
+        tcg_gen_shri_tl(t1, t1, 16);
+        tcg_gen_ext16s_tl(t1, t1);
+        gen_load_mxu_gpr(t3, opcode->D16MUL.xrc);
+        tcg_gen_ext16s_tl(t2, t3);
+        tcg_gen_shri_tl(t3, t3, 16);
+        tcg_gen_ext16s_tl(t3, t3);
+
+        switch (opcode->D16MUL.optn2) {
+        case 0: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t1, t3);
+            tcg_gen_mul_tl(t2, t0, t2);
+            break;
+        case 1: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t0, t3);
+            tcg_gen_mul_tl(t2, t0, t2);
+            break;
+        case 2: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t1, t3);
+            tcg_gen_mul_tl(t2, t1, t2);
+            break;
+        case 3: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t0, t3);
+            tcg_gen_mul_tl(t2, t1, t2);
+            break;
+        }
+        gen_store_mxu_gpr(t3, opcode->D16MUL.xra);
+        gen_store_mxu_gpr(t2, opcode->D16MUL.xrd);
+        break;
     }
 
     tcg_temp_free(t0);
     tcg_temp_free(t1);
+    tcg_temp_free(t2);
+    tcg_temp_free(t3);
 #else
     generate_exception_end(ctx, EXCP_RI);
 #endif
@@ -17975,6 +18027,7 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MXU_S32I2M:
     case OPC_MXU_S32M2I:
     case OPC_MXU_S8LDD:
+    case OPC_MXU_D16MUL:
         gen_mxu(ctx, op1);
         break;
 
-- 
2.18.0

[Qemu-devel] [PATCH 5/7] target/mips: Add MXU instruction D16MAC

[Craig Janeczek]

Adds support for emulating the D16MAC instruction.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/translate.c | 66 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 64fc6089bb..221076711d 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -366,6 +366,7 @@ enum {
     OPC_DCLO     = 0x25 | OPC_SPECIAL2,
     /* MXU */
     OPC_MXU_D16MUL = 0x08 | OPC_SPECIAL2,
+    OPC_MXU_D16MAC = 0x0A | OPC_SPECIAL2,
     OPC_MXU_S8LDD  = 0x22 | OPC_SPECIAL2,
     OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
     OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,
@@ -3806,6 +3807,17 @@ typedef union {
         uint32_t sel:2;
         uint32_t special2:6;
     } D16MUL;
+
+    struct {
+        uint32_t op:6;
+        uint32_t xra:4;
+        uint32_t xrb:4;
+        uint32_t xrc:4;
+        uint32_t xrd:4;
+        uint32_t optn2:2;
+        uint32_t aptn2:2;
+        uint32_t special2:6;
+    } D16MAC;
 } MXU_OPCODE;
 
 /* MXU Instructions */
@@ -3932,6 +3944,59 @@ static void gen_mxu(DisasContext *ctx, uint32_t opc)
         gen_store_mxu_gpr(t3, opcode->D16MUL.xra);
         gen_store_mxu_gpr(t2, opcode->D16MUL.xrd);
         break;
+
+    case OPC_MXU_D16MAC:
+        gen_load_mxu_gpr(t1, opcode->D16MAC.xrb);
+        tcg_gen_ext16s_tl(t0, t1);
+        tcg_gen_shri_tl(t1, t1, 16);
+        tcg_gen_ext16s_tl(t1, t1);
+        gen_load_mxu_gpr(t3, opcode->D16MAC.xrc);
+        tcg_gen_ext16s_tl(t2, t3);
+        tcg_gen_shri_tl(t3, t3, 16);
+        tcg_gen_ext16s_tl(t3, t3);
+
+        switch (opcode->D16MAC.optn2) {
+        case 0: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t1, t3);
+            tcg_gen_mul_tl(t2, t0, t2);
+            break;
+        case 1: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t0, t3);
+            tcg_gen_mul_tl(t2, t0, t2);
+            break;
+        case 2: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t1, t3);
+            tcg_gen_mul_tl(t2, t1, t2);
+            break;
+        case 3: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */
+            tcg_gen_mul_tl(t3, t0, t3);
+            tcg_gen_mul_tl(t2, t1, t2);
+            break;
+        }
+        gen_load_mxu_gpr(t0, opcode->D16MAC.xra);
+        gen_load_mxu_gpr(t1, opcode->D16MAC.xrd);
+
+        switch (opcode->D16MAC.aptn2) {
+        case 0:
+            tcg_gen_add_tl(t3, t0, t3);
+            tcg_gen_add_tl(t2, t1, t2);
+            break;
+        case 1:
+            tcg_gen_add_tl(t3, t0, t3);
+            tcg_gen_sub_tl(t2, t1, t2);
+            break;
+        case 2:
+            tcg_gen_sub_tl(t3, t0, t3);
+            tcg_gen_add_tl(t2, t1, t2);
+            break;
+        case 3:
+            tcg_gen_sub_tl(t3, t0, t3);
+            tcg_gen_sub_tl(t2, t1, t2);
+            break;
+        }
+        gen_store_mxu_gpr(t3, opcode->D16MAC.xra);
+        gen_store_mxu_gpr(t2, opcode->D16MAC.xrd);
+        break;
     }
 
     tcg_temp_free(t0);
@@ -18028,6 +18093,7 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MXU_S32M2I:
     case OPC_MXU_S8LDD:
     case OPC_MXU_D16MUL:
+    case OPC_MXU_D16MAC:
         gen_mxu(ctx, op1);
         break;
 
-- 
2.18.0

[Qemu-devel] [PATCH 6/7] target/mips: Add MXU instructions Q8MUL and Q8MULSU

[Craig Janeczek]

Adds support for emulating the Q8MUL and Q8MULSU instructions.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/translate.c | 75 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 221076711d..ae6cfc3d7c 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -370,6 +370,7 @@ enum {
     OPC_MXU_S8LDD  = 0x22 | OPC_SPECIAL2,
     OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
     OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,
+    OPC_MXU_Q8MUL  = 0x38 | OPC_SPECIAL2,
     /* Special */
     OPC_SDBBP    = 0x3F | OPC_SPECIAL2,
 };
@@ -3818,17 +3819,32 @@ typedef union {
         uint32_t aptn2:2;
         uint32_t special2:6;
     } D16MAC;
+
+    struct {
+        uint32_t op:6;
+        uint32_t xra:4;
+        uint32_t xrb:4;
+        uint32_t xrc:4;
+        uint32_t xrd:4;
+        uint32_t sel:2;
+        uint32_t:2;
+        uint32_t special2:6;
+    } Q8MUL;
 } MXU_OPCODE;
 
 /* MXU Instructions */
 static void gen_mxu(DisasContext *ctx, uint32_t opc)
 {
 #ifndef TARGET_MIPS64 /* Only works in 32 bit mode */
-    TCGv t0, t1, t2, t3;
+    TCGv t0, t1, t2, t3, t4, t5, t6, t7;
     t0 = tcg_temp_new();
     t1 = tcg_temp_new();
     t2 = tcg_temp_new();
     t3 = tcg_temp_new();
+    t4 = tcg_temp_new();
+    t5 = tcg_temp_new();
+    t6 = tcg_temp_new();
+    t7 = tcg_temp_new();
     MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
 
     switch (opc) {
@@ -3997,12 +4013,68 @@ static void gen_mxu(DisasContext *ctx, uint32_t opc)
         gen_store_mxu_gpr(t3, opcode->D16MAC.xra);
         gen_store_mxu_gpr(t2, opcode->D16MAC.xrd);
         break;
+
+    case OPC_MXU_Q8MUL:
+        gen_load_mxu_gpr(t3, opcode->Q8MUL.xrb);
+        gen_load_mxu_gpr(t7, opcode->Q8MUL.xrc);
+
+        if (opcode->Q8MUL.sel == 0x2) {
+            /* Q8MULSU */
+            tcg_gen_ext8s_tl(t0, t3);
+            tcg_gen_shri_tl(t3, t3, 8);
+            tcg_gen_ext8s_tl(t1, t3);
+            tcg_gen_shri_tl(t3, t3, 8);
+            tcg_gen_ext8s_tl(t2, t3);
+            tcg_gen_shri_tl(t3, t3, 8);
+            tcg_gen_ext8s_tl(t3, t3);
+        } else {
+            /* Q8MUL */
+            tcg_gen_ext8u_tl(t0, t3);
+            tcg_gen_shri_tl(t3, t3, 8);
+            tcg_gen_ext8u_tl(t1, t3);
+            tcg_gen_shri_tl(t3, t3, 8);
+            tcg_gen_ext8u_tl(t2, t3);
+            tcg_gen_shri_tl(t3, t3, 8);
+            tcg_gen_ext8u_tl(t3, t3);
+        }
+
+        tcg_gen_ext8u_tl(t4, t7);
+        tcg_gen_shri_tl(t7, t7, 8);
+        tcg_gen_ext8u_tl(t5, t7);
+        tcg_gen_shri_tl(t7, t7, 8);
+        tcg_gen_ext8u_tl(t6, t7);
+        tcg_gen_shri_tl(t7, t7, 8);
+        tcg_gen_ext8u_tl(t7, t7);
+
+        tcg_gen_mul_tl(t0, t0, t4);
+        tcg_gen_mul_tl(t1, t1, t5);
+        tcg_gen_mul_tl(t2, t2, t6);
+        tcg_gen_mul_tl(t3, t3, t7);
+
+        tcg_gen_andi_tl(t0, t0, 0xFFFF);
+        tcg_gen_andi_tl(t1, t1, 0xFFFF);
+        tcg_gen_andi_tl(t2, t2, 0xFFFF);
+        tcg_gen_andi_tl(t3, t3, 0xFFFF);
+
+        tcg_gen_shli_tl(t1, t1, 16);
+        tcg_gen_shli_tl(t3, t3, 16);
+
+        tcg_gen_or_tl(t0, t0, t1);
+        tcg_gen_or_tl(t1, t2, t3);
+
+        gen_store_mxu_gpr(t0, opcode->Q8MUL.xrd);
+        gen_store_mxu_gpr(t1, opcode->Q8MUL.xra);
+        break;
     }
 
     tcg_temp_free(t0);
     tcg_temp_free(t1);
     tcg_temp_free(t2);
     tcg_temp_free(t3);
+    tcg_temp_free(t4);
+    tcg_temp_free(t5);
+    tcg_temp_free(t6);
+    tcg_temp_free(t7);
 #else
     generate_exception_end(ctx, EXCP_RI);
 #endif
@@ -18094,6 +18166,7 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MXU_S8LDD:
     case OPC_MXU_D16MUL:
     case OPC_MXU_D16MAC:
+    case OPC_MXU_Q8MUL:
         gen_mxu(ctx, op1);
         break;
 
-- 
2.18.0

[Qemu-devel] [PATCH 7/7] target/mips: Add MXU instructions S32LDD and S32LDDR

[Craig Janeczek]

Adds support for emulating the S32LDD and S32LDDR MXU instructions.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 target/mips/translate.c | 44 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index ae6cfc3d7c..3faa95a66e 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -347,7 +347,8 @@ enum {
     OPC_MSUB     = 0x04 | OPC_SPECIAL2,
     OPC_MSUBU    = 0x05 | OPC_SPECIAL2,
     /* Loongson 2F */
-    OPC_MULT_G_2F   = 0x10 | OPC_SPECIAL2,
+    /* opcode 0x10 overlaps loongson and MXU command */
+    OPC_MULT_G_2F_MXU_S32LDD   = 0x10 | OPC_SPECIAL2,
     OPC_DMULT_G_2F  = 0x11 | OPC_SPECIAL2,
     OPC_MULTU_G_2F  = 0x12 | OPC_SPECIAL2,
     OPC_DMULTU_G_2F = 0x13 | OPC_SPECIAL2,
@@ -3798,6 +3799,15 @@ typedef union {
         uint32_t special2:6;
     } S8LDD;
 
+    struct {
+        uint32_t op:6;
+        uint32_t xra:4;
+        uint32_t s12:10;
+        uint32_t sel:1;
+        uint32_t rb:5;
+        uint32_t special2:6;
+    } S32LDD;
+
     struct {
         uint32_t op:6;
         uint32_t xra:4;
@@ -3925,6 +3935,24 @@ static void gen_mxu(DisasContext *ctx, uint32_t opc)
         gen_store_mxu_gpr(t0, opcode->S8LDD.xra);
         break;
 
+    case OPC_MULT_G_2F_MXU_S32LDD:
+        gen_load_gpr(t0, opcode->S32LDD.rb);
+
+        tcg_gen_movi_tl(t1, opcode->S32LDD.s12);
+        tcg_gen_shli_tl(t1, t1, 2);
+        if (opcode->S32LDD.s12 & 0x200) {
+            tcg_gen_ori_tl(t1, t1, 0xFFFFF000);
+        }
+        tcg_gen_add_tl(t1, t0, t1);
+        tcg_gen_qemu_ld_tl(t1, t1, ctx->mem_idx, MO_SL);
+
+        if (opcode->S32LDD.sel == 1) {
+            /* S32LDDR */
+            tcg_gen_bswap32_tl(t1, t1);
+        }
+        gen_store_mxu_gpr(t1, opcode->S32LDD.xra);
+        break;
+
     case OPC_MXU_D16MUL:
         if (opcode->D16MUL.sel == 1) {
             /* D16MULE is not supported */
@@ -4093,7 +4121,7 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
 
     switch (opc) {
     case OPC_MULT_G_2E:
-    case OPC_MULT_G_2F:
+    case OPC_MULT_G_2F_MXU_S32LDD:
     case OPC_MULTU_G_2E:
     case OPC_MULTU_G_2F:
 #if defined(TARGET_MIPS64)
@@ -4116,7 +4144,7 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
 
     switch (opc) {
     case OPC_MULT_G_2E:
-    case OPC_MULT_G_2F:
+    case OPC_MULT_G_2F_MXU_S32LDD:
         tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
         tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
         break;
@@ -18153,7 +18181,6 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         break;
     case OPC_DIV_G_2F:
     case OPC_DIVU_G_2F:
-    case OPC_MULT_G_2F:
     case OPC_MULTU_G_2F:
     case OPC_MOD_G_2F:
     case OPC_MODU_G_2F:
@@ -18170,6 +18197,15 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu(ctx, op1);
         break;
 
+    case OPC_MULT_G_2F_MXU_S32LDD:
+        /* There is an overlap of opcodes between Loongson2F and MXU */
+        if (ctx->insn_flags & INSN_LOONGSON2F) {
+            check_insn(ctx, INSN_LOONGSON2F);
+            gen_loongson_integer(ctx, op1, rd, rs, rt);
+        } else {
+            gen_mxu(ctx, op1);
+        }
+        break;
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

Re: [Qemu-devel] [PATCH 1/7] target/mips: Add MXU register support

[Richard Henderson]

On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> +/* MXU General purpose registers moves. */
> +static inline void gen_load_mxu_gpr (TCGv t, int reg)
> +{
> +    if (reg == 0)
> +        tcg_gen_movi_tl(t, 0);
> +    else
> +        tcg_gen_mov_tl(t, mxu_gpr[reg-1]);
> +}
> +
> +static inline void gen_store_mxu_gpr (TCGv t, int reg)
> +{
> +    if (reg != 0)
> +        tcg_gen_mov_tl(mxu_gpr[reg-1], t);
> +}
> +
>  /* Moves to/from shadow registers. */
>  static inline void gen_load_srsgpr (int from, int to)
>  {
> @@ -20742,6 +20767,11 @@ void mips_tcg_init(void)
>      fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
>                                         offsetof(CPUMIPSState, active_fpu.fcr31),
>                                         "fcr31");
> +
> +    for (i = 0; i < 16; i++)
> +        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
> +                                        offsetof(CPUMIPSState, active_tc.mxu_gpr[i]),
> +                                        mxuregnames[i]);
>  }

You need to fix the ./scripts/checkpatch.pl errors.
But otherwise the logic is ok.


r~

Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

[Richard Henderson]

On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> Adds support for emulating the S32I2M and S32M2I MXU instructions.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  target/mips/translate.c | 55 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 50f0cb558f..381dfad36e 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -364,6 +364,9 @@ enum {
>      OPC_CLO      = 0x21 | OPC_SPECIAL2,
>      OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
>      OPC_DCLO     = 0x25 | OPC_SPECIAL2,
> +    /* MXU */
> +    OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
> +    OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,

I haven't been able to find any documentation of the bit
layout of these instructions.  Any pointers?

> +typedef union {
> +    struct {
> +        uint32_t op:6;
> +        uint32_t xra:5;
> +        uint32_t:5;
> +        uint32_t rb:5;
> +        uint32_t:5;
> +        uint32_t special2:6;
> +    } S32I2M;
> +
> +    struct {
> +        uint32_t op:6;
> +        uint32_t xra:5;
> +        uint32_t:5;
> +        uint32_t rb:5;
> +        uint32_t:5;
> +        uint32_t special2:6;
> +    } S32M2I;
> +} MXU_OPCODE;

Do not use bitfields.  The layout differs by host compiler.
Use extract32(input, pos, len).


> +
> +/* MXU Instructions */
> +static void gen_mxu(DisasContext *ctx, uint32_t opc)
> +{
> +#ifndef TARGET_MIPS64 /* Only works in 32 bit mode */
> +    TCGv t0;
> +    t0 = tcg_temp_new();
> +    MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
> +
> +    switch (opc) {
> +    case OPC_MXU_S32I2M:
> +        gen_load_gpr(t0, opcode->S32I2M.rb);
> +        gen_store_mxu_gpr(t0, opcode->S32I2M.xra);
> +        break;
> +
> +    case OPC_MXU_S32M2I:
> +        gen_load_mxu_gpr(t0, opcode->S32M2I.xra);
> +        gen_store_gpr(t0, opcode->S32M2I.rb);
> +        break;
> +    }
> +
> +    tcg_temp_free(t0);
> +#else
> +    generate_exception_end(ctx, EXCP_RI);
> +#endif
> +}

There's nothing here (yet, I suppose) that won't compile for MIPS64.
I'd suggest avoiding ifdefs as much as possible.


r~

Re: [Qemu-devel] [PATCH 3/7] target/mips: Add MXU instruction S8LDD

[Richard Henderson]

On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> +    case OPC_MXU_S8LDD:
> +        gen_load_gpr(t0, opcode->S8LDD.rb);
> +        tcg_gen_movi_tl(t1, opcode->S8LDD.s8);
> +        tcg_gen_ext8s_tl(t1, t1);
> +        tcg_gen_add_tl(t0, t0, t1);

This is

  gen_load_gpr(t0, rb);
  tcg_gen_addi_tl(t0, t0, (int8_t)s8);

> +        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_SB);

You might want MO_UB so that you don't need

  tcg_gen_andi_tl(t1, t1, 0xff)

in several places.

Hmm.  Of course there's the two sign-extend cases that do want this.  So maybe
some more logic above the load is warranted.

> +        switch (opcode->S8LDD.optn3) {
> +        case 0: /*XRa[7:0] = tmp8 */
> +            tcg_gen_andi_tl(t1, t1, 0xFF);
> +            gen_load_mxu_gpr(t0, opcode->S8LDD.xra);
> +            tcg_gen_andi_tl(t0, t0, 0xFFFFFF00);
> +            tcg_gen_or_tl(t0, t0, t1);

  gen_load_mxu_gpr(t0, xra);
  tcg_gen_deposit_tl(t0, t0, t1, 0, 8);

> +            break;
> +        case 1: /* XRa[15:8] = tmp8 */
> +            tcg_gen_andi_tl(t1, t1, 0xFF);
> +            gen_load_mxu_gpr(t0, opcode->S8LDD.xra);
> +            tcg_gen_andi_tl(t0, t0, 0xFFFF00FF);
> +            tcg_gen_shli_tl(t1, t1, 8);
> +            tcg_gen_or_tl(t0, t0, t1);

  tcg_gen_deposit_tl(t0, t0, t1, 8, 8);

> +        case 4: /* XRa = {8'b0, tmp8, 8'b0, tmp8} */
> +            tcg_gen_andi_tl(t1, t1, 0xFF);
> +            tcg_gen_mov_tl(t0, t1);
> +            tcg_gen_shli_tl(t1, t1, 16);
> +            tcg_gen_or_tl(t0, t0, t1);
> +            break;

  tcg_gen_deposit_tl(t0, t1, t1, 16, 16);

> +        case 5: /* XRa = {tmp8, 8'b0, tmp8, 8'b0} */
> +            tcg_gen_andi_tl(t1, t1, 0xFF);
> +            tcg_gen_shli_tl(t1, t1, 8);
> +            tcg_gen_mov_tl(t0, t1);
> +            tcg_gen_shli_tl(t1, t1, 16);
> +            tcg_gen_or_tl(t0, t0, t1);

  tcg_gen_shli_tl(t1, t1, 8);
  tcg_gen_deposit_tl(t0, t1, t1, 16, 16);

> +        case 7: /* XRa = {tmp8, tmp8, tmp8, tmp8} */
> +            tcg_gen_andi_tl(t1, t1, 0xFF);
> +            tcg_gen_mov_tl(t0, t1);
> +            tcg_gen_shli_tl(t1, t1, 8);
> +            tcg_gen_or_tl(t0, t0, t1);
> +            tcg_gen_shli_tl(t1, t1, 8);
> +            tcg_gen_or_tl(t0, t0, t1);
> +            tcg_gen_shli_tl(t1, t1, 8);
> +            tcg_gen_or_tl(t0, t0, t1);

  tcg_gen_muli_tl(t0, t1, 0x01010101);


r~

Re: [Qemu-devel] [PATCH 4/7] target/mips: Add MXU instruction D16MUL

[Richard Henderson]

On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> +        gen_load_mxu_gpr(t1, opcode->D16MUL.xrb);
> +        tcg_gen_ext16s_tl(t0, t1);
> +        tcg_gen_shri_tl(t1, t1, 16);
> +        tcg_gen_ext16s_tl(t1, t1);

  tcg_gen_sextract_tl(t0, t1, 0, 16);
  tcg_gen_sextract_tl(t1, t1, 16, 16);


r~

Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

[Janeczek, Craig]

https://www.rockbox.org/wiki/pub/Main/IngenicJz4760B/jz-simd-docs.pdf

I pulled them from here. I also wrote a series of tests which I cross compiled then ran on both HW and through QEMU. Although I did not submit those tests as part of this patchset as I am unsure of how to add them into the QEMU test infrastructure.

-----Original Message-----
From: Richard Henderson <richard.henderson@linaro.org> 
Sent: Saturday, August 25, 2018 1:07 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net; amarkovic@wavecomp.com
Subject: Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> Adds support for emulating the S32I2M and S32M2I MXU instructions.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  target/mips/translate.c | 55 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> 50f0cb558f..381dfad36e 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -364,6 +364,9 @@ enum {
>      OPC_CLO      = 0x21 | OPC_SPECIAL2,
>      OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
>      OPC_DCLO     = 0x25 | OPC_SPECIAL2,
> +    /* MXU */
> +    OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
> +    OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,

I haven't been able to find any documentation of the bit layout of these instructions.  Any pointers?

> +typedef union {
> +    struct {
> +        uint32_t op:6;
> +        uint32_t xra:5;
> +        uint32_t:5;
> +        uint32_t rb:5;
> +        uint32_t:5;
> +        uint32_t special2:6;
> +    } S32I2M;
> +
> +    struct {
> +        uint32_t op:6;
> +        uint32_t xra:5;
> +        uint32_t:5;
> +        uint32_t rb:5;
> +        uint32_t:5;
> +        uint32_t special2:6;
> +    } S32M2I;
> +} MXU_OPCODE;

Do not use bitfields.  The layout differs by host compiler.
Use extract32(input, pos, len).


> +
> +/* MXU Instructions */
> +static void gen_mxu(DisasContext *ctx, uint32_t opc) { #ifndef 
> +TARGET_MIPS64 /* Only works in 32 bit mode */
> +    TCGv t0;
> +    t0 = tcg_temp_new();
> +    MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
> +
> +    switch (opc) {
> +    case OPC_MXU_S32I2M:
> +        gen_load_gpr(t0, opcode->S32I2M.rb);
> +        gen_store_mxu_gpr(t0, opcode->S32I2M.xra);
> +        break;
> +
> +    case OPC_MXU_S32M2I:
> +        gen_load_mxu_gpr(t0, opcode->S32M2I.xra);
> +        gen_store_gpr(t0, opcode->S32M2I.rb);
> +        break;
> +    }
> +
> +    tcg_temp_free(t0);
> +#else
> +    generate_exception_end(ctx, EXCP_RI); #endif }

There's nothing here (yet, I suppose) that won't compile for MIPS64.
I'd suggest avoiding ifdefs as much as possible.


r~

Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

[Janeczek, Craig]

https://github.com/MIPS/CI20_mplayer/blob/ci20_mplayer/mxu_as

Sorry I mis-read our comment. The bit layouts were pulled from this script and validated by visually examining compiled code.

-----Original Message-----
From: Janeczek, Craig 
Sent: Monday, August 27, 2018 8:15 AM
To: 'Richard Henderson' <richard.henderson@linaro.org>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net; amarkovic@wavecomp.com
Subject: RE: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

https://www.rockbox.org/wiki/pub/Main/IngenicJz4760B/jz-simd-docs.pdf

I pulled them from here. I also wrote a series of tests which I cross compiled then ran on both HW and through QEMU. Although I did not submit those tests as part of this patchset as I am unsure of how to add them into the QEMU test infrastructure.

-----Original Message-----
From: Richard Henderson <richard.henderson@linaro.org>
Sent: Saturday, August 25, 2018 1:07 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net; amarkovic@wavecomp.com
Subject: Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

On 08/24/2018 12:44 PM, Craig Janeczek via Qemu-devel wrote:
> Adds support for emulating the S32I2M and S32M2I MXU instructions.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  target/mips/translate.c | 55
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> 50f0cb558f..381dfad36e 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -364,6 +364,9 @@ enum {
>      OPC_CLO      = 0x21 | OPC_SPECIAL2,
>      OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
>      OPC_DCLO     = 0x25 | OPC_SPECIAL2,
> +    /* MXU */
> +    OPC_MXU_S32I2M = 0x2F | OPC_SPECIAL2,
> +    OPC_MXU_S32M2I = 0x2E | OPC_SPECIAL2,

I haven't been able to find any documentation of the bit layout of these instructions.  Any pointers?

> +typedef union {
> +    struct {
> +        uint32_t op:6;
> +        uint32_t xra:5;
> +        uint32_t:5;
> +        uint32_t rb:5;
> +        uint32_t:5;
> +        uint32_t special2:6;
> +    } S32I2M;
> +
> +    struct {
> +        uint32_t op:6;
> +        uint32_t xra:5;
> +        uint32_t:5;
> +        uint32_t rb:5;
> +        uint32_t:5;
> +        uint32_t special2:6;
> +    } S32M2I;
> +} MXU_OPCODE;

Do not use bitfields.  The layout differs by host compiler.
Use extract32(input, pos, len).


> +
> +/* MXU Instructions */
> +static void gen_mxu(DisasContext *ctx, uint32_t opc) { #ifndef
> +TARGET_MIPS64 /* Only works in 32 bit mode */
> +    TCGv t0;
> +    t0 = tcg_temp_new();
> +    MXU_OPCODE *opcode = (MXU_OPCODE *)&ctx->opcode;
> +
> +    switch (opc) {
> +    case OPC_MXU_S32I2M:
> +        gen_load_gpr(t0, opcode->S32I2M.rb);
> +        gen_store_mxu_gpr(t0, opcode->S32I2M.xra);
> +        break;
> +
> +    case OPC_MXU_S32M2I:
> +        gen_load_mxu_gpr(t0, opcode->S32M2I.xra);
> +        gen_store_gpr(t0, opcode->S32M2I.rb);
> +        break;
> +    }
> +
> +    tcg_temp_free(t0);
> +#else
> +    generate_exception_end(ctx, EXCP_RI); #endif }

There's nothing here (yet, I suppose) that won't compile for MIPS64.
I'd suggest avoiding ifdefs as much as possible.


r~

Re: [Qemu-devel] [PATCH 1/7] target/mips: Add MXU register support

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Friday, August 24, 2018 9:44 PM
>
> Subject: [PATCH 1/7] target/mips: Add MXU register support
> 
> This commit makes the MXU registers and the helper functions for
> reading/writing to them. This is required for full MXU instruction
> support.

Hi, Craig,

The term "helper function" is good in general terminology sense, however, in QEMU terminology, it is used for something else (see op_helper.c for examples), and not for the case similar to the functions in this patch. Your functions generate "inline" code, in QEMU terminology, which is opposite to "helper" functions. Just don't use the word "helper" in the commit message. Use "wrapper", "utility", or similar word.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH 1/7] target/mips: Add MXU register support

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Friday, August 24, 2018 9:44 PM
>
> Subject: [PATCH 1/7] target/mips: Add MXU register support
> 
>This commit makes the MXU registers and the helper functions for
> reading/writing to them. This is required for full MXU instruction
> support.

> Signed-off-by: Craig Janeczek <jancraig@amazon.com>

Also, Craig,

This patch will cause compiler errors if QEMU is built with clang, with this patch as the last one. Clang will complain about unused functions. This will affect bisect algorithms. you should move definition of the functions to the patches where they are used for the first time in the series.

Otherwise, overall, we want the content of your series integrated, you just need to improve/correct the code.

Regards,
Aleksandar

Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

[Aleksandar Markovic]

> Craig Janeczek via Qemu-devel wrote:
>
> I also wrote a series of tests which I cross compiled then ran on
> both HW and through QEMU. Although I did not submit those tests
> as part of this patchset as I am unsure of how to add them into the 
> QEMU test infrastructure.

Alex,

Here we have another case of someone having - and wanting to add to QEMU - some MIPS tests.

I gather they are very similar to the tests in:

tests/tcg/mips/mips32-dsp, and
tests/tcg/mips/mips32-dspr2.

However, you say something is wrong with them (but we are using them for testings of DSP and DSP R2 ASE for MIPS CPUs that have such feature).

Can you please tell us:

- What should we do to existing DSP and DSP R2 to fit into QEMU test infrastructure? 

- What would be the guidance for Craig to integrate new tests that he wrote into QEMU (which are I suppose of similar nature to DSP tests)?

Many thanks!

Aleksandar

Re: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I

[Aleksandar Markovic]

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Monday, August 27, 2018 2:22 PM
> 
> Subject: RE: [Qemu-devel] [PATCH 2/7] target/mips: Add MXU instructions S32I2M and S32M2I
> 
> https://github.com/MIPS/CI20_mplayer/blob/ci20_mplayer/mxu_as
> 
> Sorry I mis-read our comment. The bit layouts were pulled from
> this script and validated by visually examining compiled code.

I think Richard brings to your attention that the structure definitions using bit fields may yield different results on little and big endian hosts, which is certainly the situation you want to avooid, and recommends using extract32(), or similar QEMU feature, instead.

Aleksandar