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

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

[Craig Janeczek]

This patch set begins to add MXU instruction support for mips
emulation.

Craig Janeczek (9):
  target/mips: Introduce MXU registers
  target/mips: Add all MXU opcodes
  target/mips: Split mips instruction handling
  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       |   2 +
 target/mips/mips-defs.h |   1 +
 target/mips/translate.c | 598 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 600 insertions(+), 1 deletion(-)

-- 
2.18.0

[Qemu-devel] [PATCH v4 1/9] target/mips: Introduce MXU registers

[Craig Janeczek]

Define and initialize the 16 MXU registers.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - NA
 v2
    - NA
 v3
    - Initial patch, split out from prior first patch
 v4
    - fixed reg name alignment
    - added braces around init for loop
    - Split mxu_CR out of the mxu_gpr array

 target/mips/cpu.h       |  2 ++
 target/mips/translate.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 009202cf64..ff356f529b 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -170,6 +170,8 @@ struct TCState {
         MSACSR_FS_MASK)
 
     float_status msa_fp_status;
+    target_ulong mxu_gpr[15];
+    target_ulong mxu_cr;
 };
 
 typedef struct CPUMIPSState CPUMIPSState;
diff --git a/target/mips/translate.c b/target/mips/translate.c
index bdd880bb77..19b90c8735 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1398,6 +1398,10 @@ 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[15];
+static TCGv mxu_CR;
+
 #include "exec/gen-icount.h"
 
 #define gen_helper_0e0i(name, arg) do {                           \
@@ -1517,6 +1521,11 @@ 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) {                                               \
@@ -20742,6 +20751,17 @@ void mips_tcg_init(void)
     fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
+
+    for (i = 0; i < 15; i++) {
+        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
+                                        offsetof(CPUMIPSState,
+                                                 active_tc.mxu_gpr[i]),
+                                        mxuregnames[i]);
+    }
+
+    mxu_CR = tcg_global_mem_new(cpu_env,
+                                offsetof(CPUMIPSState, active_tc.mxu_cr),
+                                "MXU_CR");
 }
 
 #include "translate_init.inc.c"
-- 
2.18.0

[Qemu-devel] [PATCH v4 2/9] target/mips: Add all MXU opcodes

[Craig Janeczek]

Adds all MXU opcodes to the opcode enum.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - NA
 v2
    - NA
 v3
    - Initial patch, split out from prior first patch
 v4
    - separate MXU opcodes into their own enum

 target/mips/translate.c | 60 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 19b90c8735..a598f45558 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -368,6 +368,66 @@ enum {
     OPC_SDBBP    = 0x3F | OPC_SPECIAL2,
 };
 
+enum {
+    /* MXU */
+    OPC_MXU_S32MADD  = 0x00 | OPC_SPECIAL2,
+    OPC_MXU_S32MADDU = 0x01 | OPC_SPECIAL2,
+    OPC_MXU_D16MAX   = 0x03 | OPC_SPECIAL2,
+    OPC_MXU_S32MSUB  = 0x04 | OPC_SPECIAL2,
+    OPC_MXU_S32MSUBU = 0x05 | OPC_SPECIAL2,
+    OPC_MXU_D16AVG   = 0x06 | OPC_SPECIAL2,
+    OPC_MXU_D16CPS   = 0x07 | OPC_SPECIAL2,
+    OPC_MXU_D16MUL   = 0x08 | OPC_SPECIAL2,
+    OPC_MXU_D16MULF  = 0x09 | OPC_SPECIAL2,
+    OPC_MXU_D16MAC   = 0x0A | OPC_SPECIAL2,
+    OPC_MXU_D16MACF  = 0x0B | OPC_SPECIAL2,
+    OPC_MXU_D16MADL  = 0x0C | OPC_SPECIAL2,
+    OPC_MXU_S16MAD   = 0x0D | OPC_SPECIAL2,
+    OPC_MXU_Q16ADD   = 0x0E | OPC_SPECIAL2,
+    OPC_MXU_D16MACE  = 0x0F | OPC_SPECIAL2,
+    OPC_MXU_S32LDD   = 0x10 | OPC_SPECIAL2,
+    OPC_MXU_S32STD   = 0x11 | OPC_SPECIAL2,
+    OPC_MXU_S32LDDV  = 0x12 | OPC_SPECIAL2,
+    OPC_MXU_S32STDV  = 0x13 | OPC_SPECIAL2,
+    OPC_MXU_S32LDI   = 0x14 | OPC_SPECIAL2,
+    OPC_MXU_S32SDI   = 0x15 | OPC_SPECIAL2,
+    OPC_MXU_S32LDIV  = 0x16 | OPC_SPECIAL2,
+    OPC_MXU_S32SDIV  = 0x17 | OPC_SPECIAL2,
+    OPC_MXU_D32ADD   = 0x18 | OPC_SPECIAL2,
+    OPC_MXU_D32ACC   = 0x19 | OPC_SPECIAL2,
+    OPC_MXU_Q16ACC   = 0x1B | OPC_SPECIAL2,
+    OPC_MXU_Q8ADDE   = 0x1C | OPC_SPECIAL2,
+    OPC_MXU_Q8ACCE   = 0x1D | OPC_SPECIAL2,
+    OPC_MXU_S8LDD    = 0x22 | OPC_SPECIAL2,
+    OPC_MXU_S8STD    = 0x23 | OPC_SPECIAL2,
+    OPC_MXU_S8LDI    = 0x24 | OPC_SPECIAL2,
+    OPC_MXU_S8SDI    = 0x25 | OPC_SPECIAL2,
+    OPC_MXU_S32EXTR  = 0x26 | OPC_SPECIAL2,
+    OPC_MXU_D32SARW  = 0x27 | OPC_SPECIAL2,
+    OPC_MXU_LXB      = 0x28 | OPC_SPECIAL2,
+    OPC_MXU_S16LDD   = 0x2A | OPC_SPECIAL2,
+    OPC_MXU_S16STD   = 0x2B | OPC_SPECIAL2,
+    OPC_MXU_S16LDI   = 0x2C | OPC_SPECIAL2,
+    OPC_MXU_S16SDI   = 0x2D | OPC_SPECIAL2,
+    OPC_MXU_S32M2I   = 0x2E | OPC_SPECIAL2,
+    OPC_MXU_S32I2M   = 0x2F | OPC_SPECIAL2,
+    OPC_MXU_D32SLL   = 0x30 | OPC_SPECIAL2,
+    OPC_MXU_D32SLR   = 0x31 | OPC_SPECIAL2,
+    OPC_MXU_D32SARL  = 0x32 | OPC_SPECIAL2,
+    OPC_MXU_D32SAR   = 0x33 | OPC_SPECIAL2,
+    OPC_MXU_Q16SLL   = 0x34 | OPC_SPECIAL2,
+    OPC_MXU_Q16SLR   = 0x35 | OPC_SPECIAL2,
+    OPC_MXU_D32SLLV  = 0x36 | OPC_SPECIAL2,
+    OPC_MXU_Q16SAR   = 0x37 | OPC_SPECIAL2,
+    OPC_MXU_Q8MUL    = 0x38 | OPC_SPECIAL2,
+    OPC_MXU_Q8MOVZ   = 0x39 | OPC_SPECIAL2,
+    OPC_MXU_Q8MAC    = 0x3A | OPC_SPECIAL2,
+    OPC_MXU_Q16SCOP  = 0x3B | OPC_SPECIAL2,
+    OPC_MXU_Q8MADL   = 0x3C | OPC_SPECIAL2,
+    OPC_MXU_S32SFL   = 0x3D | OPC_SPECIAL2,
+    OPC_MXU_Q8SAD    = 0x3E | OPC_SPECIAL2,
+};
+
 /* Special3 opcodes */
 #define MASK_SPECIAL3(op)  MASK_OP_MAJOR(op) | (op & 0x3F)
 
-- 
2.18.0

[Qemu-devel] [PATCH v4 3/9] target/mips: Split mips instruction handling

[Craig Janeczek]

Splits the instruction handling switch statement from the original
legacy code.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - NA
 v2
    - NA
 v3
    - NA
 v4
    - Initial patch

 target/mips/mips-defs.h |  1 +
 target/mips/translate.c | 28 +++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index d239069975..5a409757f0 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -50,6 +50,7 @@
 #define   ASE_SMARTMIPS 0x00400000
 #define   ASE_MICROMIPS 0x00800000
 #define   ASE_MSA       0x01000000
+#define   ASE_MXU       0x02000000
 
 /* Chip specific instructions. */
 #define		INSN_LOONGSON2E  0x20000000
diff --git a/target/mips/translate.c b/target/mips/translate.c
index a598f45558..53d896ebf9 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -17855,6 +17855,28 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
+static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
+{
+    int rs, rt, rd;
+    uint32_t op1;
+
+    rs = (ctx->opcode >> 21) & 0x1f;
+    rt = (ctx->opcode >> 16) & 0x1f;
+    rd = (ctx->opcode >> 11) & 0x1f;
+
+    op1 = MASK_SPECIAL2(ctx->opcode);
+
+    switch (op1) {
+    case OPC_MUL:
+        gen_arith(ctx, op1, rd, rs, rt);
+        break;
+    default:            /* Invalid */
+        MIPS_INVAL("special2_mxu");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    }
+}
+
 static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
 {
     int rs, rt, rd;
@@ -19836,7 +19858,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         decode_opc_special(env, ctx);
         break;
     case OPC_SPECIAL2:
-        decode_opc_special2_legacy(env, ctx);
+        if (ctx->insn_flags & ASE_MXU) {
+            decode_opc_special2_mxu(env, ctx);
+        } else {
+            decode_opc_special2_legacy(env, ctx);
+        }
         break;
     case OPC_SPECIAL3:
         decode_opc_special3(env, ctx);
-- 
2.18.0

[Qemu-devel] [PATCH v4 4/9] target/mips: Add MXU instructions S32I2M and S32M2I

[Craig Janeczek]

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

Adds support for emulating the S32I2M and S32M2I MXU instructions.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - initial patch
 v2
    - Fix checkpatch.pl errors
    - remove mips64 ifdef
    - changed bitfield usage to extract32
    - squashed register addition patch into this one
 v3
    - Split register addition and opcode enum definition into seperate patches
    - Split gen_mxu function into command specific gen_mxu_<ins> functions
 v4
    - changed MXU register utility functions to take in unsigned argument
    - Created seperate utility functions for MXU_CR
    - Moved ins handling to mxu specific switch statement

 target/mips/translate.c | 83 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 53d896ebf9..41081ee066 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1619,6 +1619,34 @@ 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, unsigned int reg)
+{
+    if (reg == 0) {
+        tcg_gen_movi_tl(t, 0);
+    } else if (reg <= 15) {
+        tcg_gen_mov_tl(t, mxu_gpr[reg - 1]);
+    }
+}
+
+static inline void gen_store_mxu_gpr(TCGv t, unsigned int reg)
+{
+    if (reg > 0 && reg <= 15) {
+        tcg_gen_mov_tl(mxu_gpr[reg - 1], t);
+    }
+}
+
+/* MXU control register moves. */
+static inline void gen_load_mxu_cr(TCGv t)
+{
+    tcg_gen_mov_tl(t, mxu_CR);
+}
+
+static inline void gen_store_mxu_cr(TCGv t)
+{
+    tcg_gen_mov_tl(mxu_CR, t);
+}
+
 /* Moves to/from shadow registers. */
 static inline void gen_load_srsgpr (int from, int to)
 {
@@ -3807,6 +3835,51 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
     }
 }
 
+/* MXU Instructions */
+
+/* S32I2M XRa, rb - Register move from GRF to XRF */
+static void gen_mxu_s32i2m(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0;
+    uint32_t xra, rb;
+
+    t0 = tcg_temp_new();
+
+    xra = extract32(ctx->opcode, 6, 5);
+    rb = extract32(ctx->opcode, 16, 5);
+
+    gen_load_gpr(t0, rb);
+    if (xra <= 15) {
+        gen_store_mxu_gpr(t0, xra);
+    } else if (xra == 16) {
+        gen_store_mxu_cr(t0);
+    }
+
+    tcg_temp_free(t0);
+}
+
+/* S32M2I XRa, rb - Register move from XRF to GRF */
+static void gen_mxu_s32m2i(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0;
+    uint32_t xra, rb;
+
+    t0 = tcg_temp_new();
+
+    xra = extract32(ctx->opcode, 6, 5);
+    rb = extract32(ctx->opcode, 16, 5);
+
+    if (xra <= 15) {
+        gen_load_mxu_gpr(t0, xra);
+    } else if (xra == 16) {
+        gen_load_mxu_cr(t0);
+    }
+
+    gen_store_gpr(t0, rb);
+
+    tcg_temp_free(t0);
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -17870,6 +17943,15 @@ static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
     case OPC_MUL:
         gen_arith(ctx, op1, rd, rs, rt);
         break;
+
+    case OPC_MXU_S32I2M:
+        gen_mxu_s32i2m(ctx, op1);
+        break;
+
+    case OPC_MXU_S32M2I:
+        gen_mxu_s32m2i(ctx, op1);
+        break;
+
     default:            /* Invalid */
         MIPS_INVAL("special2_mxu");
         generate_exception_end(ctx, EXCP_RI);
@@ -17909,6 +17991,7 @@ 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_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

[Qemu-devel] [PATCH v4 5/9] target/mips: Add MXU instruction S8LDD

[Craig Janeczek]

Adds support for emulating the S8LDD MXU instruction.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - initial patch
 v2
    - changed bitfield usage to extract32
    - used deposit_tl instructions instead of shift and bitmask
 v3
    - Split gen_mxu function into command specific gen_mxu_<ins> functions
 v4
    -Add and use MXU_OPTN3_PTN #defines
    -Add check for MXUEN

 target/mips/translate.c | 98 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 41081ee066..cfd25c3abe 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1462,6 +1462,8 @@ static TCGv_i64 msa_wr_d[64];
 static TCGv mxu_gpr[15];
 static TCGv mxu_CR;
 
+#define MXUEN 0x01
+
 #include "exec/gen-icount.h"
 
 #define gen_helper_0e0i(name, arg) do {                           \
@@ -3837,6 +3839,16 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
 
 /* MXU Instructions */
 
+/* MXU operand getting patterns OPTN3 */
+#define MXU_OPTN3_PTN0  0
+#define MXU_OPTN3_PTN1  1
+#define MXU_OPTN3_PTN2  2
+#define MXU_OPTN3_PTN3  3
+#define MXU_OPTN3_PTN4  4
+#define MXU_OPTN3_PTN5  5
+#define MXU_OPTN3_PTN6  6
+#define MXU_OPTN3_PTN7  7
+
 /* S32I2M XRa, rb - Register move from GRF to XRF */
 static void gen_mxu_s32i2m(DisasContext *ctx, uint32_t opc)
 {
@@ -3880,6 +3892,88 @@ static void gen_mxu_s32m2i(DisasContext *ctx, uint32_t opc)
     tcg_temp_free(t0);
 }
 
+/* S8LDD XRa, rb, S8, OPTN3 - Load a byte from memory to XRF */
+static void gen_mxu_s8ldd(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0, t1;
+    TCGLabel *l0;
+    uint32_t xra, s8, optn3, rb;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+
+    l0 = gen_new_label();
+
+    xra = extract32(ctx->opcode, 6, 4);
+    s8 = extract32(ctx->opcode, 10, 8);
+    optn3 = extract32(ctx->opcode, 18, 3);
+    rb = extract32(ctx->opcode, 21, 5);
+
+    gen_load_mxu_cr(t0);
+    tcg_gen_andi_tl(t0, t0, MXUEN);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
+
+    gen_load_gpr(t0, rb);
+    tcg_gen_addi_tl(t0, t0, (int8_t)s8);
+    switch (optn3) {
+    /*XRa[7:0] = tmp8 */
+    case MXU_OPTN3_PTN0:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        gen_load_mxu_gpr(t0, xra);
+        tcg_gen_deposit_tl(t0, t0, t1, 0, 8);
+        break;
+    /* XRa[15:8] = tmp8 */
+    case MXU_OPTN3_PTN1:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        gen_load_mxu_gpr(t0, xra);
+        tcg_gen_deposit_tl(t0, t0, t1, 8, 8);
+        break;
+    /* XRa[23:16] = tmp8 */
+    case MXU_OPTN3_PTN2:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        gen_load_mxu_gpr(t0, xra);
+        tcg_gen_deposit_tl(t0, t0, t1, 16, 8);
+        break;
+    /* XRa[31:24] = tmp8 */
+    case MXU_OPTN3_PTN3:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        gen_load_mxu_gpr(t0, xra);
+        tcg_gen_deposit_tl(t0, t0, t1, 24, 8);
+        break;
+    /* XRa = {8'b0, tmp8, 8'b0, tmp8} */
+    case MXU_OPTN3_PTN4:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        tcg_gen_deposit_tl(t0, t1, t1, 16, 16);
+        break;
+    /* XRa = {tmp8, 8'b0, tmp8, 8'b0} */
+    case MXU_OPTN3_PTN5:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        tcg_gen_shli_tl(t1, t1, 8);
+        tcg_gen_deposit_tl(t0, t1, t1, 16, 16);
+        break;
+    /* XRa = {{8{sign of tmp8}}, tmp8, {8{sign of tmp8}}, tmp8} */
+    case MXU_OPTN3_PTN6:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_SB);
+        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;
+    /* XRa = {tmp8, tmp8, tmp8, tmp8} */
+    case MXU_OPTN3_PTN7:
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        tcg_gen_deposit_tl(t1, t1, t1, 8, 8);
+        tcg_gen_deposit_tl(t0, t1, t1, 16, 16);
+        break;
+    }
+    gen_store_mxu_gpr(t0, xra);
+
+    gen_set_label(l0);
+
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -17952,6 +18046,10 @@ static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_s32m2i(ctx, op1);
         break;
 
+    case OPC_MXU_S8LDD:
+        gen_mxu_s8ldd(ctx, op1);
+        break;
+
     default:            /* Invalid */
         MIPS_INVAL("special2_mxu");
         generate_exception_end(ctx, EXCP_RI);
-- 
2.18.0

[Qemu-devel] [PATCH v4 6/9] target/mips: Add MXU instruction D16MUL

[Craig Janeczek]

Adds support for emulating the D16MUL instruction.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - initial patch
 v2
    - changed bitfield usage to extract32
    - used sextract_tl instructions instead of shift and ext
 v3
    - Split gen_mxu function into command specific gen_mxu_<ins> functions
 v4
    - Add and use MXU_OPTN2_ #defines
    - Add check for MXUEN

 target/mips/translate.c | 70 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index cfd25c3abe..0241f1fca4 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3849,6 +3849,12 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
 #define MXU_OPTN3_PTN6  6
 #define MXU_OPTN3_PTN7  7
 
+/* MXU operand getting patterns */
+#define MXU_OPTN2_WW    0
+#define MXU_OPTN2_LW    1
+#define MXU_OPTN2_HW    2
+#define MXU_OPTN2_XW    3
+
 /* S32I2M XRa, rb - Register move from GRF to XRF */
 static void gen_mxu_s32i2m(DisasContext *ctx, uint32_t opc)
 {
@@ -3974,6 +3980,66 @@ static void gen_mxu_s8ldd(DisasContext *ctx, uint32_t opc)
     tcg_temp_free(t1);
 }
 
+/* D16MUL XRa, XRb, XRc, XRd, OPTN2 - Signed 16 bit pattern multiplication */
+static void gen_mxu_d16mul(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0, t1, t2, t3;
+    TCGLabel *l0;
+    uint32_t xra, xrb, xrc, xrd, optn2;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+    t2 = tcg_temp_new();
+    t3 = tcg_temp_new();
+
+    l0 = gen_new_label();
+
+    xra = extract32(ctx->opcode, 6, 4);
+    xrb = extract32(ctx->opcode, 10, 4);
+    xrc = extract32(ctx->opcode, 14, 4);
+    xrd = extract32(ctx->opcode, 18, 4);
+    optn2 = extract32(ctx->opcode, 22, 2);
+
+    gen_load_mxu_cr(t0);
+    tcg_gen_andi_tl(t0, t0, MXUEN);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
+
+    gen_load_mxu_gpr(t1, xrb);
+    tcg_gen_sextract_tl(t0, t1, 0, 16);
+    tcg_gen_sextract_tl(t1, t1, 16, 16);
+    gen_load_mxu_gpr(t3, xrc);
+    tcg_gen_sextract_tl(t2, t3, 0, 16);
+    tcg_gen_sextract_tl(t3, t3, 16, 16);
+
+    switch (optn2) {
+    case MXU_OPTN2_WW: /* 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 MXU_OPTN2_LW: /* 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 MXU_OPTN2_HW: /* 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 MXU_OPTN2_XW: /* 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, xra);
+    gen_store_mxu_gpr(t2, xrd);
+
+    gen_set_label(l0);
+
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+    tcg_temp_free(t2);
+    tcg_temp_free(t3);
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -18050,6 +18116,10 @@ static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_s8ldd(ctx, op1);
         break;
 
+    case OPC_MXU_D16MUL:
+        gen_mxu_d16mul(ctx, op1);
+        break;
+
     default:            /* Invalid */
         MIPS_INVAL("special2_mxu");
         generate_exception_end(ctx, EXCP_RI);
-- 
2.18.0

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

[Craig Janeczek]

Adds support for emulating the D16MAC instruction.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - initial patch
 v2
    - changed bitfield usage to extract32
    - used sextract_tl instructions instead of shift and ext
 v3
    - Split gen_mxu function into command specific gen_mxu_<ins> functions
 v4
    - Use MXU_OPTN2_ #defines
    - Add and use MXU_APTN2_ #defines
    - correct multi line comment format
    - Add check for MXUEN

 target/mips/translate.c | 95 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 0241f1fca4..ea6484e2db 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3855,6 +3855,12 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
 #define MXU_OPTN2_HW    2
 #define MXU_OPTN2_XW    3
 
+/* MXU acumulate patterns */
+#define MXU_APTN2_AA    0
+#define MXU_APTN2_AS    1
+#define MXU_APTN2_SA    2
+#define MXU_APTN2_SS    3
+
 /* S32I2M XRa, rb - Register move from GRF to XRF */
 static void gen_mxu_s32i2m(DisasContext *ctx, uint32_t opc)
 {
@@ -4040,6 +4046,91 @@ static void gen_mxu_d16mul(DisasContext *ctx, uint32_t opc)
     tcg_temp_free(t3);
 }
 
+/*
+ * D16MAC XRa, XRb, XRc, XRd, APTN2, OPTN2
+ * Signed 16 bit pattern multiply and accumulate
+ */
+static void gen_mxu_d16mac(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0, t1, t2, t3;
+    TCGLabel *l0;
+    uint32_t xra, xrb, xrc, xrd, optn2, aptn2;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+    t2 = tcg_temp_new();
+    t3 = tcg_temp_new();
+
+    l0 = gen_new_label();
+
+    xra = extract32(ctx->opcode, 6, 4);
+    xrb = extract32(ctx->opcode, 10, 4);
+    xrc = extract32(ctx->opcode, 14, 4);
+    xrd = extract32(ctx->opcode, 18, 4);
+    optn2 = extract32(ctx->opcode, 22, 2);
+    aptn2 = extract32(ctx->opcode, 24, 2);
+
+    gen_load_mxu_cr(t0);
+    tcg_gen_andi_tl(t0, t0, MXUEN);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
+
+    gen_load_mxu_gpr(t1, xrb);
+    tcg_gen_sextract_tl(t0, t1, 0, 16);
+    tcg_gen_sextract_tl(t1, t1, 16, 16);
+    gen_load_mxu_gpr(t3, xrc);
+    tcg_gen_sextract_tl(t2, t3, 0, 16);
+    tcg_gen_sextract_tl(t3, t3, 16, 16);
+
+    switch (optn2) {
+    case MXU_OPTN2_WW: /* 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 MXU_OPTN2_LW: /* 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 MXU_OPTN2_HW: /* 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 MXU_OPTN2_XW: /* 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, xra);
+    gen_load_mxu_gpr(t1, xrd);
+
+    switch (aptn2) {
+    case MXU_APTN2_AA:
+        tcg_gen_add_tl(t3, t0, t3);
+        tcg_gen_add_tl(t2, t1, t2);
+        break;
+    case MXU_APTN2_AS:
+        tcg_gen_add_tl(t3, t0, t3);
+        tcg_gen_sub_tl(t2, t1, t2);
+        break;
+    case MXU_APTN2_SA:
+        tcg_gen_sub_tl(t3, t0, t3);
+        tcg_gen_add_tl(t2, t1, t2);
+        break;
+    case MXU_APTN2_SS:
+        tcg_gen_sub_tl(t3, t0, t3);
+        tcg_gen_sub_tl(t2, t1, t2);
+        break;
+    }
+    gen_store_mxu_gpr(t3, xra);
+    gen_store_mxu_gpr(t2, xrd);
+
+    gen_set_label(l0);
+
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+    tcg_temp_free(t2);
+    tcg_temp_free(t3);
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -18120,6 +18211,10 @@ static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_d16mul(ctx, op1);
         break;
 
+    case OPC_MXU_D16MAC:
+        gen_mxu_d16mac(ctx, op1);
+        break;
+
     default:            /* Invalid */
         MIPS_INVAL("special2_mxu");
         generate_exception_end(ctx, EXCP_RI);
-- 
2.18.0

[Qemu-devel] [PATCH v4 8/9] 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>
---
 v1
    - initial patch
 v2
    - changed bitfield usage to extract32
 v3
    - Split gen_mxu function into command specific gen_mxu_<ins> functions
 v4
    - Add check for MXUEN

 target/mips/translate.c | 95 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index ea6484e2db..31c7342261 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4131,6 +4131,97 @@ static void gen_mxu_d16mac(DisasContext *ctx, uint32_t opc)
     tcg_temp_free(t3);
 }
 
+/* Q8MUL XRa, XRb, XRc, XRd - Parallel unsigned 8 bit pattern multiply */
+/* Q8MULSU XRa, XRb, XRc, XRd - Parallel signed 8 bit pattern multiply */
+static void gen_mxu_q8mul(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0, t1, t2, t3, t4, t5, t6, t7;
+    TCGLabel *l0;
+    uint32_t xra, xrb, xrc, xrd, sel;
+
+    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();
+
+    l0 = gen_new_label();
+
+    xra = extract32(ctx->opcode, 6, 4);
+    xrb = extract32(ctx->opcode, 10, 4);
+    xrc = extract32(ctx->opcode, 14, 4);
+    xrd = extract32(ctx->opcode, 18, 4);
+    sel = extract32(ctx->opcode, 22, 4);
+
+    gen_load_mxu_cr(t0);
+    tcg_gen_andi_tl(t0, t0, MXUEN);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
+
+    gen_load_mxu_gpr(t3, xrb);
+    gen_load_mxu_gpr(t7, xrc);
+
+    if (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, xrd);
+    gen_store_mxu_gpr(t1, xra);
+
+    gen_set_label(l0);
+
+    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);
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -18215,6 +18306,10 @@ static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_d16mac(ctx, op1);
         break;
 
+    case OPC_MXU_Q8MUL:
+        gen_mxu_q8mul(ctx, op1);
+        break;
+
     default:            /* Invalid */
         MIPS_INVAL("special2_mxu");
         generate_exception_end(ctx, EXCP_RI);
-- 
2.18.0

[Qemu-devel] [PATCH v4 9/9] 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>
---
 v1
    - initial patch
 v2
    - changed bitfield usage to extract32
 v3
    - Split gen_mxu function into command specific gen_mxu_<ins> functions
 v4
    - Add check for MXUEN

 target/mips/translate.c | 49 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 31c7342261..297f913d36 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4222,6 +4222,51 @@ static void gen_mxu_q8mul(DisasContext *ctx, uint32_t opc)
     tcg_temp_free(t7);
 }
 
+/* S32LDD XRa, rb, S12 - Load a word from memory to XRF
+ * S32LDDR XRa, rb, S12 - Load a word from memory to XRF,
+ *                        reversed byte sequence */
+static void gen_mxu_s32ldd(DisasContext *ctx, uint32_t opc)
+{
+    TCGv t0, t1;
+    TCGLabel *l0;
+    uint32_t xra, s12, sel, rb;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+
+    l0 = gen_new_label();
+
+    xra = extract32(ctx->opcode, 6, 4);
+    s12 = extract32(ctx->opcode, 10, 10);
+    sel = extract32(ctx->opcode, 20, 1);
+    rb = extract32(ctx->opcode, 21, 5);
+
+    gen_load_mxu_cr(t0);
+    tcg_gen_andi_tl(t0, t0, MXUEN);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
+
+    gen_load_gpr(t0, rb);
+
+    tcg_gen_movi_tl(t1, s12);
+    tcg_gen_shli_tl(t1, t1, 2);
+    if (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 (sel == 1) {
+        /* S32LDDR */
+        tcg_gen_bswap32_tl(t1, t1);
+    }
+    gen_store_mxu_gpr(t1, xra);
+
+    gen_set_label(l0);
+
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 /* Godson integer instructions */
 static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
                                  int rd, int rs, int rt)
@@ -18310,6 +18355,10 @@ static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_q8mul(ctx, op1);
         break;
 
+    case OPC_MXU_S32LDD:
+        gen_mxu_s32ldd(ctx, op1);
+        break;
+
     default:            /* Invalid */
         MIPS_INVAL("special2_mxu");
         generate_exception_end(ctx, EXCP_RI);
-- 
2.18.0

Re: [Qemu-devel] [PATCH v4 5/9] target/mips: Add MXU instruction S8LDD

[Aleksandar Markovic]

Hi, Craig,

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
> 
> Subject: [PATCH v4 5/9] target/mips: Add MXU instruction S8LDD

> Adds support for emulating the S8LDD MXU instruction.

> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  v1
>     - initial patch
>  v2
>     - changed bitfield usage to extract32
>     - used deposit_tl instructions instead of shift and bitmask
>  v3
>     - Split gen_mxu function into command specific gen_mxu_<ins> functions
>  v4
>     -Add and use MXU_OPTN3_PTN #defines
>     -Add check for MXUEN

>  target/mips/translate.c | 98 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)

> diff --git a/target/mips/translate.c b/target/mips/translate.c

> +    TCGv t0, t1;
> +    TCGLabel *l0;
> +    uint32_t xra, s8, optn3, rb;
> +
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +
> +    l0 = gen_new_label();
> +
> +    xra = extract32(ctx->opcode, 6, 4);
> +    s8 = extract32(ctx->opcode, 10, 8);
> +    optn3 = extract32(ctx->opcode, 18, 3);
> +    rb = extract32(ctx->opcode, 21, 5);
> +
> +    gen_load_mxu_cr(t0);
> +    tcg_gen_andi_tl(t0, t0, MXUEN);
> +    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
> +
> +    gen_load_gpr(t0, rb);
> +    tcg_gen_addi_tl(t0, t0, (int8_t)s8);

I am not sure if this works as desired, with respect to branching. In order to survive branching, tcg variables must be initialized with tcg_temp_local_new(), rather than with tcg_tem_new(). Please retest, and amend if needed.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v4 3/9] target/mips: Split mips instruction handling

[Aleksandar Markovic]

Hi, Craig,

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v4 3/9] target/mips: Split mips instruction handling
> 
> Splits the instruction handling switch statement from the original
> legacy code.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  v1
>     - NA
>  v2
>     - NA
>  v3
>     - NA
>  v4
>     - Initial patch
> 
>  target/mips/mips-defs.h |  1 +
>  target/mips/translate.c | 28 +++++++++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
> index d239069975..5a409757f0 100644
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -50,6 +50,7 @@
>  #define   ASE_SMARTMIPS 0x00400000
>  #define   ASE_MICROMIPS 0x00800000
>  #define   ASE_MSA       0x01000000
> +#define   ASE_MXU       0x02000000
> 
>  /* Chip specific instructions. */
>  #define                INSN_LOONGSON2E  0x20000000
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index a598f45558..53d896ebf9 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -17855,6 +17855,28 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
>      }
>  }
> 
> +static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    int rs, rt, rd;
> +    uint32_t op1;
> +
> +    rs = (ctx->opcode >> 21) & 0x1f;
> +    rt = (ctx->opcode >> 16) & 0x1f;
> +    rd = (ctx->opcode >> 11) & 0x1f;
> +
> +    op1 = MASK_SPECIAL2(ctx->opcode);
> +
> +    switch (op1) {
> +    case OPC_MUL:
> +        gen_arith(ctx, op1, rd, rs, rt);
> +        break;
> +    default:            /* Invalid */
> +        MIPS_INVAL("special2_mxu");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +

This (case OPC_MUL) just looks very odd to me. Why would OPC_MUL somehow be supposed to be included here? Is there any documentation to support this? For example of other kind: OPC_MADD is not included in this switch, but there is an OPC_MADD equivalent in MXU. At the same time, there is an OPC_MUL equivalent in MXU too.

This looks to me as a very unclear opcode organization. Too bad the MXU documentation that you linked to doesn't have opcode specifications. Xburst base set documentation would be very helpful, but there is no such doc to my knowledge.

Sincerely,
Aleksandar

>  static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>  {
>      int rs, rt, rd;
> @@ -19836,7 +19858,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
>          decode_opc_special(env, ctx);
>          break;
>      case OPC_SPECIAL2:
> -        decode_opc_special2_legacy(env, ctx);
> +        if (ctx->insn_flags & ASE_MXU) {
> +            decode_opc_special2_mxu(env, ctx);
> +        } else {
> +            decode_opc_special2_legacy(env, ctx);
> +        }
>          break;
>      case OPC_SPECIAL3:
>          decode_opc_special3(env, ctx);
> --
> 2.18.0
> 

Re: [Qemu-devel] [PATCH v4 2/9] target/mips: Add all MXU opcodes

[Aleksandar Markovic]

> 
> ________________________________________
> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v4 2/9] target/mips: Add all MXU opcodes
> 
> Adds all MXU opcodes to the opcode enum.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  v1
>     - NA
>  v2
>     - NA
>  v3
>     - Initial patch, split out from prior first patch
>  v4
>     - separate MXU opcodes into their own enum
> 
>  target/mips/translate.c | 60 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 




> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 19b90c8735..a598f45558 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c

Hi, Craig,

There are 56 opcodes defined here. However, there are 114 MXU instructions in the MXU doc. Some other bits are used for coding missing instructions. Even in your patches, it seems, you handle some instructions that are not listed here. Is there any way to have here the complete list of opcodes, even if this involves some more complex extraction operations?

Thanks,
Aleksandar


> @@ -368,6 +368,66 @@ enum {
>      OPC_SDBBP    = 0x3F | OPC_SPECIAL2,
>  };
> +enum {
> +    /* MXU */
> +    OPC_MXU_S32MADD  = 0x00 | OPC_SPECIAL2,
> +    OPC_MXU_S32MADDU = 0x01 | OPC_SPECIAL2,
> +    OPC_MXU_D16MAX   = 0x03 | OPC_SPECIAL2,
> +    OPC_MXU_S32MSUB  = 0x04 | OPC_SPECIAL2,
> +    OPC_MXU_S32MSUBU = 0x05 | OPC_SPECIAL2,
> +    OPC_MXU_D16AVG   = 0x06 | OPC_SPECIAL2,
> +    OPC_MXU_D16CPS   = 0x07 | OPC_SPECIAL2,
> +    OPC_MXU_D16MUL   = 0x08 | OPC_SPECIAL2,
> +    OPC_MXU_D16MULF  = 0x09 | OPC_SPECIAL2,
> +    OPC_MXU_D16MAC   = 0x0A | OPC_SPECIAL2,
> +    OPC_MXU_D16MACF  = 0x0B | OPC_SPECIAL2,
> +    OPC_MXU_D16MADL  = 0x0C | OPC_SPECIAL2,
> +    OPC_MXU_S16MAD   = 0x0D | OPC_SPECIAL2,
> +    OPC_MXU_Q16ADD   = 0x0E | OPC_SPECIAL2,
> +    OPC_MXU_D16MACE  = 0x0F | OPC_SPECIAL2,
> +    OPC_MXU_S32LDD   = 0x10 | OPC_SPECIAL2,
> +    OPC_MXU_S32STD   = 0x11 | OPC_SPECIAL2,
> +    OPC_MXU_S32LDDV  = 0x12 | OPC_SPECIAL2,
> +    OPC_MXU_S32STDV  = 0x13 | OPC_SPECIAL2,
> +    OPC_MXU_S32LDI   = 0x14 | OPC_SPECIAL2,
> +    OPC_MXU_S32SDI   = 0x15 | OPC_SPECIAL2,
> +    OPC_MXU_S32LDIV  = 0x16 | OPC_SPECIAL2,
> +    OPC_MXU_S32SDIV  = 0x17 | OPC_SPECIAL2,
> +    OPC_MXU_D32ADD   = 0x18 | OPC_SPECIAL2,
> +    OPC_MXU_D32ACC   = 0x19 | OPC_SPECIAL2,
> +    OPC_MXU_Q16ACC   = 0x1B | OPC_SPECIAL2,
> +    OPC_MXU_Q8ADDE   = 0x1C | OPC_SPECIAL2,
> +    OPC_MXU_Q8ACCE   = 0x1D | OPC_SPECIAL2,
> +    OPC_MXU_S8LDD    = 0x22 | OPC_SPECIAL2,
> +    OPC_MXU_S8STD    = 0x23 | OPC_SPECIAL2,
> +    OPC_MXU_S8LDI    = 0x24 | OPC_SPECIAL2,
> +    OPC_MXU_S8SDI    = 0x25 | OPC_SPECIAL2,
> +    OPC_MXU_S32EXTR  = 0x26 | OPC_SPECIAL2,
> +    OPC_MXU_D32SARW  = 0x27 | OPC_SPECIAL2,
> +    OPC_MXU_LXB      = 0x28 | OPC_SPECIAL2,
> +    OPC_MXU_S16LDD   = 0x2A | OPC_SPECIAL2,
> +    OPC_MXU_S16STD   = 0x2B | OPC_SPECIAL2,
> +    OPC_MXU_S16LDI   = 0x2C | OPC_SPECIAL2,
> +    OPC_MXU_S16SDI   = 0x2D | OPC_SPECIAL2,
> +    OPC_MXU_S32M2I   = 0x2E | OPC_SPECIAL2,
> +    OPC_MXU_S32I2M   = 0x2F | OPC_SPECIAL2,
> +    OPC_MXU_D32SLL   = 0x30 | OPC_SPECIAL2,
> +    OPC_MXU_D32SLR   = 0x31 | OPC_SPECIAL2,
> +    OPC_MXU_D32SARL  = 0x32 | OPC_SPECIAL2,
> +    OPC_MXU_D32SAR   = 0x33 | OPC_SPECIAL2,
> +    OPC_MXU_Q16SLL   = 0x34 | OPC_SPECIAL2,
> +    OPC_MXU_Q16SLR   = 0x35 | OPC_SPECIAL2,
> +    OPC_MXU_D32SLLV  = 0x36 | OPC_SPECIAL2,
> +    OPC_MXU_Q16SAR   = 0x37 | OPC_SPECIAL2,
> +    OPC_MXU_Q8MUL    = 0x38 | OPC_SPECIAL2,
> +    OPC_MXU_Q8MOVZ   = 0x39 | OPC_SPECIAL2,
> +    OPC_MXU_Q8MAC    = 0x3A | OPC_SPECIAL2,
> +    OPC_MXU_Q16SCOP  = 0x3B | OPC_SPECIAL2,
> +    OPC_MXU_Q8MADL   = 0x3C | OPC_SPECIAL2,
> +    OPC_MXU_S32SFL   = 0x3D | OPC_SPECIAL2,
> +    OPC_MXU_Q8SAD    = 0x3E | OPC_SPECIAL2,
> +};
> +
>  /* Special3 opcodes */
>  #define MASK_SPECIAL3(op)  MASK_OP_MAJOR(op) | (op & 0x3F)

Re: [Qemu-devel] [PATCH v4 3/9] target/mips: Split mips instruction handling

[Janeczek, Craig]

To clarify the OPC_MUL here is not an MXU instruction, this is the original OPC_MUL that was in the special2 instruction set. The inclusion of this instruction in this switch statement is due to the suggested method of splitting up the mxu commands instruction handling switch statement from the original special2 commands. Since there is no MXU command with the opcode suffix of 0x02 there was not an instruction collision. Your other example is not correct as there is an MXU instruction sharing the opcode suffix 0x00 (OPC_MXU_S32MADD) therefore the original OPC_MUL would not be used.

Remember that I did not arbitrarily make this instruction mapping, I just implemented the list of MXU opcodes. The confusion stems from the fact that these opcodes overlap with pre-existing instructions and do not consistently map original instruction to MXU instruction. 

I have not been able to find a document to back this up. The only evidence I have is the existence of the OPC_MUL instruction in an MXU compiled binary.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Friday, August 31, 2018 2:40 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net; Petar Jovanovic <pjovanovic@wavecomp.com>; Richard Henderson <richard.henderson@linaro.org>
Subject: Re: [PATCH v4 3/9] target/mips: Split mips instruction handling

Hi, Craig,

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v4 3/9] target/mips: Split mips instruction handling
> 
> Splits the instruction handling switch statement from the original 
> legacy code.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  v1
>     - NA
>  v2
>     - NA
>  v3
>     - NA
>  v4
>     - Initial patch
> 
>  target/mips/mips-defs.h |  1 +
>  target/mips/translate.c | 28 +++++++++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h index 
> d239069975..5a409757f0 100644
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -50,6 +50,7 @@
>  #define   ASE_SMARTMIPS 0x00400000
>  #define   ASE_MICROMIPS 0x00800000
>  #define   ASE_MSA       0x01000000
> +#define   ASE_MXU       0x02000000
> 
>  /* Chip specific instructions. */
>  #define                INSN_LOONGSON2E  0x20000000
> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> a598f45558..53d896ebf9 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -17855,6 +17855,28 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
>      }
>  }
> 
> +static void decode_opc_special2_mxu(CPUMIPSState *env, DisasContext 
> +*ctx) {
> +    int rs, rt, rd;
> +    uint32_t op1;
> +
> +    rs = (ctx->opcode >> 21) & 0x1f;
> +    rt = (ctx->opcode >> 16) & 0x1f;
> +    rd = (ctx->opcode >> 11) & 0x1f;
> +
> +    op1 = MASK_SPECIAL2(ctx->opcode);
> +
> +    switch (op1) {
> +    case OPC_MUL:
> +        gen_arith(ctx, op1, rd, rs, rt);
> +        break;
> +    default:            /* Invalid */
> +        MIPS_INVAL("special2_mxu");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +

This (case OPC_MUL) just looks very odd to me. Why would OPC_MUL somehow be supposed to be included here? Is there any documentation to support this? For example of other kind: OPC_MADD is not included in this switch, but there is an OPC_MADD equivalent in MXU. At the same time, there is an OPC_MUL equivalent in MXU too.

This looks to me as a very unclear opcode organization. Too bad the MXU documentation that you linked to doesn't have opcode specifications. Xburst base set documentation would be very helpful, but there is no such doc to my knowledge.

Sincerely,
Aleksandar

>  static void decode_opc_special2_legacy(CPUMIPSState *env, 
> DisasContext *ctx)  {
>      int rs, rt, rd;
> @@ -19836,7 +19858,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
>          decode_opc_special(env, ctx);
>          break;
>      case OPC_SPECIAL2:
> -        decode_opc_special2_legacy(env, ctx);
> +        if (ctx->insn_flags & ASE_MXU) {
> +            decode_opc_special2_mxu(env, ctx);
> +        } else {
> +            decode_opc_special2_legacy(env, ctx);
> +        }
>          break;
>      case OPC_SPECIAL3:
>          decode_opc_special3(env, ctx);
> --
> 2.18.0
> 

Re: [Qemu-devel] [PATCH v4 2/9] target/mips: Add all MXU opcodes

[Janeczek, Craig]

It is possible but due to the non-uniform nature of the bit fields which indicate the instructions the mask used might end up being instruction specific. This would lead to a tiered approach where the current enum would be the top level, then each group under there might have its own enum, mask and switch statement.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Friday, August 31, 2018 2:59 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net; Petar Jovanovic <pjovanovic@wavecomp.com>; Richard Henderson <richard.henderson@linaro.org>
Subject: Re: [PATCH v4 2/9] target/mips: Add all MXU opcodes

> 
> ________________________________________
> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v4 2/9] target/mips: Add all MXU opcodes
> 
> Adds all MXU opcodes to the opcode enum.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>
> ---
>  v1
>     - NA
>  v2
>     - NA
>  v3
>     - Initial patch, split out from prior first patch
>  v4
>     - separate MXU opcodes into their own enum
> 
>  target/mips/translate.c | 60 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 




> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> 19b90c8735..a598f45558 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c

Hi, Craig,

There are 56 opcodes defined here. However, there are 114 MXU instructions in the MXU doc. Some other bits are used for coding missing instructions. Even in your patches, it seems, you handle some instructions that are not listed here. Is there any way to have here the complete list of opcodes, even if this involves some more complex extraction operations?

Thanks,
Aleksandar


> @@ -368,6 +368,66 @@ enum {
>      OPC_SDBBP    = 0x3F | OPC_SPECIAL2,
>  };
> +enum {
> +    /* MXU */
> +    OPC_MXU_S32MADD  = 0x00 | OPC_SPECIAL2,
> +    OPC_MXU_S32MADDU = 0x01 | OPC_SPECIAL2,
> +    OPC_MXU_D16MAX   = 0x03 | OPC_SPECIAL2,
> +    OPC_MXU_S32MSUB  = 0x04 | OPC_SPECIAL2,
> +    OPC_MXU_S32MSUBU = 0x05 | OPC_SPECIAL2,
> +    OPC_MXU_D16AVG   = 0x06 | OPC_SPECIAL2,
> +    OPC_MXU_D16CPS   = 0x07 | OPC_SPECIAL2,
> +    OPC_MXU_D16MUL   = 0x08 | OPC_SPECIAL2,
> +    OPC_MXU_D16MULF  = 0x09 | OPC_SPECIAL2,
> +    OPC_MXU_D16MAC   = 0x0A | OPC_SPECIAL2,
> +    OPC_MXU_D16MACF  = 0x0B | OPC_SPECIAL2,
> +    OPC_MXU_D16MADL  = 0x0C | OPC_SPECIAL2,
> +    OPC_MXU_S16MAD   = 0x0D | OPC_SPECIAL2,
> +    OPC_MXU_Q16ADD   = 0x0E | OPC_SPECIAL2,
> +    OPC_MXU_D16MACE  = 0x0F | OPC_SPECIAL2,
> +    OPC_MXU_S32LDD   = 0x10 | OPC_SPECIAL2,
> +    OPC_MXU_S32STD   = 0x11 | OPC_SPECIAL2,
> +    OPC_MXU_S32LDDV  = 0x12 | OPC_SPECIAL2,
> +    OPC_MXU_S32STDV  = 0x13 | OPC_SPECIAL2,
> +    OPC_MXU_S32LDI   = 0x14 | OPC_SPECIAL2,
> +    OPC_MXU_S32SDI   = 0x15 | OPC_SPECIAL2,
> +    OPC_MXU_S32LDIV  = 0x16 | OPC_SPECIAL2,
> +    OPC_MXU_S32SDIV  = 0x17 | OPC_SPECIAL2,
> +    OPC_MXU_D32ADD   = 0x18 | OPC_SPECIAL2,
> +    OPC_MXU_D32ACC   = 0x19 | OPC_SPECIAL2,
> +    OPC_MXU_Q16ACC   = 0x1B | OPC_SPECIAL2,
> +    OPC_MXU_Q8ADDE   = 0x1C | OPC_SPECIAL2,
> +    OPC_MXU_Q8ACCE   = 0x1D | OPC_SPECIAL2,
> +    OPC_MXU_S8LDD    = 0x22 | OPC_SPECIAL2,
> +    OPC_MXU_S8STD    = 0x23 | OPC_SPECIAL2,
> +    OPC_MXU_S8LDI    = 0x24 | OPC_SPECIAL2,
> +    OPC_MXU_S8SDI    = 0x25 | OPC_SPECIAL2,
> +    OPC_MXU_S32EXTR  = 0x26 | OPC_SPECIAL2,
> +    OPC_MXU_D32SARW  = 0x27 | OPC_SPECIAL2,
> +    OPC_MXU_LXB      = 0x28 | OPC_SPECIAL2,
> +    OPC_MXU_S16LDD   = 0x2A | OPC_SPECIAL2,
> +    OPC_MXU_S16STD   = 0x2B | OPC_SPECIAL2,
> +    OPC_MXU_S16LDI   = 0x2C | OPC_SPECIAL2,
> +    OPC_MXU_S16SDI   = 0x2D | OPC_SPECIAL2,
> +    OPC_MXU_S32M2I   = 0x2E | OPC_SPECIAL2,
> +    OPC_MXU_S32I2M   = 0x2F | OPC_SPECIAL2,
> +    OPC_MXU_D32SLL   = 0x30 | OPC_SPECIAL2,
> +    OPC_MXU_D32SLR   = 0x31 | OPC_SPECIAL2,
> +    OPC_MXU_D32SARL  = 0x32 | OPC_SPECIAL2,
> +    OPC_MXU_D32SAR   = 0x33 | OPC_SPECIAL2,
> +    OPC_MXU_Q16SLL   = 0x34 | OPC_SPECIAL2,
> +    OPC_MXU_Q16SLR   = 0x35 | OPC_SPECIAL2,
> +    OPC_MXU_D32SLLV  = 0x36 | OPC_SPECIAL2,
> +    OPC_MXU_Q16SAR   = 0x37 | OPC_SPECIAL2,
> +    OPC_MXU_Q8MUL    = 0x38 | OPC_SPECIAL2,
> +    OPC_MXU_Q8MOVZ   = 0x39 | OPC_SPECIAL2,
> +    OPC_MXU_Q8MAC    = 0x3A | OPC_SPECIAL2,
> +    OPC_MXU_Q16SCOP  = 0x3B | OPC_SPECIAL2,
> +    OPC_MXU_Q8MADL   = 0x3C | OPC_SPECIAL2,
> +    OPC_MXU_S32SFL   = 0x3D | OPC_SPECIAL2,
> +    OPC_MXU_Q8SAD    = 0x3E | OPC_SPECIAL2,
> +};
> +
>  /* Special3 opcodes */
>  #define MASK_SPECIAL3(op)  MASK_OP_MAJOR(op) | (op & 0x3F)

Re: [Qemu-devel] [PATCH v4 0/9] Add limited MXU instruction support

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
>
> Subject: [PATCH v4 0/9] Add limited MXU instruction support

> This patch set begins to add MXU instruction support for mips emulation.

Hi, Craig,

I mentioned some time ago that we planned to revamp translate.c. Related to that, I would like to tweak your whole series (this will be most of the time moving code from place to place, and basically leaving your code intact, just reorganized differently; I'll try to collect most of the MXU code in a separate file). To be efficient (avoid long explanation and email threads), I would like to do it just after you submit v5 - I would then submit v6, and you would continue working on such modified series, creating v7 by yourself etc., if needed.

In relation to that, when do you plan to submit v5?

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v4 3/9] target/mips: Split mips instruction handling

[Aleksandar Markovic]

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Tuesday, September 4, 2018 4:44 PM
>
> Subject: RE: [PATCH v4 3/9] target/mips: Split mips instruction handling
>
> To clarify the OPC_MUL here is not an MXU instruction, this is the original OPC_MUL that was in the special2 instruction set. The inclusion of this instruction in this switch statement is due to the suggested method of splitting up the mxu commands instruction handling switch statement from the original special2 commands.

There are five more cases where current SPECIAL2 instructions occupy free slots in  MXU opcode scheme:

    /* Loongson 2F */
    OPC_MODU_G_2F   = 0x1e | OPC_SPECIAL2,
    OPC_DMODU_G_2F  = 0x1f | OPC_SPECIAL2,
    /* Misc */
    OPC_CLZ      = 0x20 | OPC_SPECIAL2,
    OPC_CLO      = 0x21 | OPC_SPECIAL2,
    /* Special */
    OPC_SDBBP = 0x3F | OPC_SPECIAL2,

What to do with them? Should they be treated like OPC_MUL? Can you do the same binary check as for OPC_MUL? Is there a confirmation in Ingenic gcc/asm source for all these cases?

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v4 3/9] target/mips: Split mips instruction handling

[Aleksandar Markovic]

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Tuesday, September 4, 2018 4:44 PM
>
> Subject: RE: [PATCH v4 3/9] target/mips: Split mips instruction handling
>
> To clarify the OPC_MUL here is not an MXU instruction, this is the original OPC_MUL that was in the special2 instruction set. The inclusion of this instruction in this switch statement is due to the suggested method of splitting up the mxu commands instruction handling switch statement from the original special2 commands.

In any case, handling OPC_MUL (and others similar cases if it turns out to be needed) in such way should be in a separate patch in this series, and backed by, at the least, references to the Ingenic source code of gcc/asm or similar utilities.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v4 0/9] Add limited MXU instruction support

[Janeczek, Craig]

I plan on getting back to this development effort next week.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Wednesday, September 5, 2018 9:36 AM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v4 0/9] Add limited MXU instruction support

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Thursday, August 30, 2018 9:30 PM
>
> Subject: [PATCH v4 0/9] Add limited MXU instruction support

> This patch set begins to add MXU instruction support for mips emulation.

Hi, Craig,

I mentioned some time ago that we planned to revamp translate.c. Related to that, I would like to tweak your whole series (this will be most of the time moving code from place to place, and basically leaving your code intact, just reorganized differently; I'll try to collect most of the MXU code in a separate file). To be efficient (avoid long explanation and email threads), I would like to do it just after you submit v5 - I would then submit v6, and you would continue working on such modified series, creating v7 by yourself etc., if needed.

In relation to that, when do you plan to submit v5?

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v4 1/9] target/mips: Introduce MXU registers

[Richard Henderson]

On 08/30/2018 12:30 PM, Craig Janeczek via Qemu-devel wrote:
> +static const char * const mxuregnames[] = {
> +    "XR1", "XR2",  "XR3",  "XR4",  "XR5",  "XR6",  "XR7",  "XR8",
> +    "XR9", "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "XR16",
> +};

XR16 is unused.  Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v4 4/9] target/mips: Add MXU instructions S32I2M and S32M2I

[Richard Henderson]

On 08/30/2018 12:30 PM, Craig Janeczek via Qemu-devel wrote:
> +/* S32I2M XRa, rb - Register move from GRF to XRF */
> +static void gen_mxu_s32i2m(DisasContext *ctx, uint32_t opc)
> +{
> +    TCGv t0;
> +    uint32_t xra, rb;
> +
> +    t0 = tcg_temp_new();
> +
> +    xra = extract32(ctx->opcode, 6, 5);
> +    rb = extract32(ctx->opcode, 16, 5);
> +
> +    gen_load_gpr(t0, rb);
> +    if (xra <= 15) {
> +        gen_store_mxu_gpr(t0, xra);
> +    } else if (xra == 16) {
> +        gen_store_mxu_cr(t0);
> +    }

else...?  Illegal instruction / reserved operand fault?
Surely it is not treated as a nop (although stranger things happen).

> +static void gen_mxu_s32m2i(DisasContext *ctx, uint32_t opc)
> +{
> +    TCGv t0;
> +    uint32_t xra, rb;
> +
> +    t0 = tcg_temp_new();
> +
> +    xra = extract32(ctx->opcode, 6, 5);
> +    rb = extract32(ctx->opcode, 16, 5);
> +
> +    if (xra <= 15) {
> +        gen_load_mxu_gpr(t0, xra);
> +    } else if (xra == 16) {
> +        gen_load_mxu_cr(t0);
> +    }
> +
> +    gen_store_gpr(t0, rb);

Likewise.  Although this one will crash qemu, because t0 will be used by
gen_store_gpr without being initialized.

> @@ -17909,6 +17991,7 @@ 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_CLO:

Avoid random whitespace changes.


r~

Re: [Qemu-devel] [PATCH v4 5/9] target/mips: Add MXU instruction S8LDD

[Richard Henderson]

On 08/30/2018 12:30 PM, Craig Janeczek via Qemu-devel wrote:
> +    gen_load_mxu_cr(t0);
> +    tcg_gen_andi_tl(t0, t0, MXUEN);
> +    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);

Probably MXUEN should be included in env->hflags, and therefore
tested via ctx->hflags.  (Which also means ending a TB after a
write to MCR, which can change this value).

The documentation says that if MXUEN is unset the result is
"unpredictable".  What does real hardware do?  Does it really
treat the instruction as a nop?  Does it raise an exception?

Otherwise another possibility for "unpredictable" is to simply
execute the instruction.  Which would certainly be the easiest
implementation...


r~

Re: [Qemu-devel] [PATCH v4 5/9] target/mips: Add MXU instruction S8LDD

[Richard Henderson]

On 08/31/2018 06:39 AM, Aleksandar Markovic wrote:
>> +    gen_load_mxu_cr(t0);
>> +    tcg_gen_andi_tl(t0, t0, MXUEN);
>> +    tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXUEN, l0);
>> +
>> +    gen_load_gpr(t0, rb);
>> +    tcg_gen_addi_tl(t0, t0, (int8_t)s8);
> 
> I am not sure if this works as desired, with respect to branching. In order to survive branching, tcg variables must be initialized with tcg_temp_local_new(), rather than with tcg_tem_new(). Please retest, and amend if needed.

There is no value live across the branch here.

T0 is used before the branch, yes, but it is also reset
immediately via the gen_load_gpr after the branch.

This is fine.


r~