[Qemu-devel] [PATCH 0/9] target/mips: Limited support for R5900 multimedia instructions

[Qemu-devel] [PATCH 0/9] target/mips: Limited support for R5900 multimedia instructions

[Fredrik Noring]

This series introduces limited support for R5900 multimedia instructions
(MMIs) by implementing and testing PCPYLD, PCPYUD, LQ and SQ.

32 128-bit multimedia registers (MMRs) are introduced. They are split into
two 64-bit halves: the lower halves are the GPRs and the upper halves are
accessible by the R5900-specific multimedia instructions.

The MMIs are only available with TARGET_MIPS64 since 64-bit GPRs are
required.

The relation between SQ and RDHWR may need to be revisited. I'm hoping to
discuss the details with Maciej.

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

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

in addition successfully completing the R5900 test suite

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

Fredrik Noring (9):
  target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
  target/mips: Introduce 32 R5900 128-bit multimedia registers
  target/mips: Support the R5900 PCPYLD multimedia instruction
  target/mips: Support the R5900 PCPYUD multimedia instruction
  target/mips: Support the R5900 LQ multimedia instruction
  target/mips: Support the R5900 SQ multimedia instruction
  tests/tcg/mips: Test R5900 multimedia instructions PCPYUD and PCPYLD
  tests/tcg/mips: Test R5900 multimedia instruction LQ
  tests/tcg/mips: Test R5900 multimedia instruction SQ

 target/mips/cpu.h                     |   2 +
 target/mips/translate.c               | 180 +++++++++++++++++++++++---
 tests/tcg/mips/mipsn32r5900/Makefile  |  27 ++++
 tests/tcg/mips/mipsn32r5900/lq.c      | 111 ++++++++++++++++
 tests/tcg/mips/mipsn32r5900/pcpyuld.c |  46 +++++++
 tests/tcg/mips/mipsn32r5900/sq.c      | 105 +++++++++++++++
 6 files changed, 452 insertions(+), 19 deletions(-)
 create mode 100644 tests/tcg/mips/mipsn32r5900/Makefile
 create mode 100644 tests/tcg/mips/mipsn32r5900/lq.c
 create mode 100644 tests/tcg/mips/mipsn32r5900/pcpyuld.c
 create mode 100644 tests/tcg/mips/mipsn32r5900/sq.c

-- 
2.19.2

[Qemu-devel] [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

[Fredrik Noring]

The R5900 MMIs operate on 128-bit registers that will be split into
two halves: lower 64-bit GPRs and upper 64-bit MMRs. The MMIs will
therefore be left unimplemented in 32-bit mode with the o32 ABI.

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 057aaf9a44..a538351032 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27218,6 +27218,7 @@ static void decode_opc_special3_legacy(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
+#if defined(TARGET_MIPS64)
 static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opc = MASK_MMI0(ctx->opcode);
@@ -27351,6 +27352,7 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
         break;
     }
 }
+#endif /* defined(TARGET_MIPS64) */
 
 static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
 {
@@ -27360,18 +27362,6 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
     int rd = extract32(ctx->opcode, 11, 5);
 
     switch (opc) {
-    case MMI_OPC_CLASS_MMI0:
-        decode_mmi0(env, ctx);
-        break;
-    case MMI_OPC_CLASS_MMI1:
-        decode_mmi1(env, ctx);
-        break;
-    case MMI_OPC_CLASS_MMI2:
-        decode_mmi2(env, ctx);
-        break;
-    case MMI_OPC_CLASS_MMI3:
-        decode_mmi3(env, ctx);
-        break;
     case MMI_OPC_MULT1:
     case MMI_OPC_MULTU1:
     case MMI_OPC_MADD:
@@ -27392,6 +27382,7 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_MFHI1:
         gen_HILO1_tx79(ctx, opc, rd);
         break;
+#if defined(TARGET_MIPS64)
     case MMI_OPC_PLZCW:         /* TODO: MMI_OPC_PLZCW */
     case MMI_OPC_PMFHL:         /* TODO: MMI_OPC_PMFHL */
     case MMI_OPC_PMTHL:         /* TODO: MMI_OPC_PMTHL */
@@ -27403,6 +27394,19 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_PSRAW:         /* TODO: MMI_OPC_PSRAW */
         generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_CLASS_MMI */
         break;
+    case MMI_OPC_CLASS_MMI0:
+        decode_mmi0(env, ctx);
+        break;
+    case MMI_OPC_CLASS_MMI1:
+        decode_mmi1(env, ctx);
+        break;
+    case MMI_OPC_CLASS_MMI2:
+        decode_mmi2(env, ctx);
+        break;
+    case MMI_OPC_CLASS_MMI3:
+        decode_mmi3(env, ctx);
+        break;
+#endif
     default:
         MIPS_INVAL("TX79 MMI class");
         generate_exception_end(ctx, EXCP_RI);
-- 
2.19.2

[Qemu-devel] [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia registers

[Fredrik Noring]

The 32 R5900 128-bit MMRs are split into two 64-bit halves: the lower
halves are the GPRs and the upper halves are accessible by the R5900-
specific multimedia instructions.

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 target/mips/cpu.h       |  2 ++
 target/mips/translate.c | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 03c03fd8c6..9ff4a68d90 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -180,6 +180,8 @@ struct TCState {
 #define MXU_CR_RD_EN    1
 #define MXU_CR_MXU_EN   0
 
+    /* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
+    uint64_t mmr[32];
 };
 
 typedef struct CPUMIPSState CPUMIPSState;
diff --git a/target/mips/translate.c b/target/mips/translate.c
index a538351032..9d5150ec8b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2455,7 +2455,10 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
 
-#if !defined(TARGET_MIPS64)
+#if defined(TARGET_MIPS64)
+/* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
+static TCGv_i64 cpu_mmr[32];
+#else
 /* MXU registers */
 static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
 static TCGv mxu_CR;
@@ -29785,7 +29788,14 @@ void mips_tcg_init(void)
     fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
-#if !defined(TARGET_MIPS64)
+#if defined(TARGET_MIPS64)
+    cpu_mmr[0] = NULL;
+    for (i = 1; i < 32; i++)
+        cpu_mmr[i] = tcg_global_mem_new_i64(cpu_env,
+                                            offsetof(CPUMIPSState,
+                                                     active_tc.mmr[i]),
+                                            regnames[i]);
+#else
     for (i = 0; i < NUMBER_OF_MXU_REGISTERS - 1; i++) {
         mxu_gpr[i] = tcg_global_mem_new(cpu_env,
                                         offsetof(CPUMIPSState,
-- 
2.19.2

[Qemu-devel] [PATCH 3/9] target/mips: Support the R5900 PCPYLD multimedia instruction

[Fredrik Noring]

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 9d5150ec8b..40faf9cb36 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27293,11 +27293,34 @@ static void decode_mmi1(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
+static void gen_mmi_pcpyld(DisasContext *ctx, int rd, int rs, int rt)
+{
+    if (rd != 0) {
+        if (rs != 0) {
+            tcg_gen_mov_i64(cpu_mmr[rd], cpu_gpr[rs]);
+        } else {
+            tcg_gen_movi_i64(cpu_mmr[rd], 0);
+        }
+
+        if (rt != 0) {
+            tcg_gen_mov_i64(cpu_gpr[rd], cpu_gpr[rt]);
+        } else {
+            tcg_gen_movi_i64(cpu_gpr[rd], 0);
+        }
+    }
+}
+
 static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opc = MASK_MMI2(ctx->opcode);
+    int rs = extract32(ctx->opcode, 21, 5);
+    int rt = extract32(ctx->opcode, 16, 5);
+    int rd = extract32(ctx->opcode, 11, 5);
 
     switch (opc) {
+    case MMI_OPC_2_PCPYLD:
+        gen_mmi_pcpyld(ctx, rd, rs, rt);
+        break;
     case MMI_OPC_2_PMADDW:    /* TODO: MMI_OPC_2_PMADDW */
     case MMI_OPC_2_PSLLVW:    /* TODO: MMI_OPC_2_PSLLVW */
     case MMI_OPC_2_PSRLVW:    /* TODO: MMI_OPC_2_PSRLVW */
@@ -27307,7 +27330,6 @@ static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_2_PINTH:     /* TODO: MMI_OPC_2_PINTH */
     case MMI_OPC_2_PMULTW:    /* TODO: MMI_OPC_2_PMULTW */
     case MMI_OPC_2_PDIVW:     /* TODO: MMI_OPC_2_PDIVW */
-    case MMI_OPC_2_PCPYLD:    /* TODO: MMI_OPC_2_PCPYLD */
     case MMI_OPC_2_PMADDH:    /* TODO: MMI_OPC_2_PMADDH */
     case MMI_OPC_2_PHMADH:    /* TODO: MMI_OPC_2_PHMADH */
     case MMI_OPC_2_PAND:      /* TODO: MMI_OPC_2_PAND */
-- 
2.19.2

[Qemu-devel] [PATCH 4/9] target/mips: Support the R5900 PCPYUD multimedia instruction

[Fredrik Noring]

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 40faf9cb36..8c350729bc 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27351,11 +27351,34 @@ static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
     }
 }
 
+static void gen_mmi_pcpyud(DisasContext *ctx, int rd, int rs, int rt)
+{
+    if (rd != 0) {
+        if (rs != 0) {
+            tcg_gen_mov_i64(cpu_gpr[rd], cpu_mmr[rs]);
+        } else {
+            tcg_gen_movi_i64(cpu_gpr[rd], 0);
+        }
+
+        if (rt != 0) {
+            tcg_gen_mov_i64(cpu_mmr[rd], cpu_mmr[rt]);
+        } else {
+            tcg_gen_movi_i64(cpu_mmr[rd], 0);
+        }
+    }
+}
+
 static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opc = MASK_MMI3(ctx->opcode);
+    int rs = extract32(ctx->opcode, 21, 5);
+    int rt = extract32(ctx->opcode, 16, 5);
+    int rd = extract32(ctx->opcode, 11, 5);
 
     switch (opc) {
+    case MMI_OPC_3_PCPYUD:
+        gen_mmi_pcpyud(ctx, rd, rs, rt);
+        break;
     case MMI_OPC_3_PMADDUW:    /* TODO: MMI_OPC_3_PMADDUW */
     case MMI_OPC_3_PSRAVW:     /* TODO: MMI_OPC_3_PSRAVW */
     case MMI_OPC_3_PMTHI:      /* TODO: MMI_OPC_3_PMTHI */
@@ -27363,7 +27386,6 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_3_PINTEH:     /* TODO: MMI_OPC_3_PINTEH */
     case MMI_OPC_3_PMULTUW:    /* TODO: MMI_OPC_3_PMULTUW */
     case MMI_OPC_3_PDIVUW:     /* TODO: MMI_OPC_3_PDIVUW */
-    case MMI_OPC_3_PCPYUD:     /* TODO: MMI_OPC_3_PCPYUD */
     case MMI_OPC_3_POR:        /* TODO: MMI_OPC_3_POR */
     case MMI_OPC_3_PNOR:       /* TODO: MMI_OPC_3_PNOR */
     case MMI_OPC_3_PEXCH:      /* TODO: MMI_OPC_3_PEXCH */
-- 
2.19.2

[Qemu-devel] [PATCH 5/9] target/mips: Support the R5900 LQ multimedia instruction

[Fredrik Noring]

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 8c350729bc..79505bb6c2 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2626,6 +2626,17 @@ static inline void gen_store_gpr (TCGv t, int reg)
         tcg_gen_mov_tl(cpu_gpr[reg], t);
 }
 
+#if defined(TARGET_MIPS64)
+/* 128-bit multimedia register moves. */
+static inline void gen_store_mmr(TCGv_i64 hi, TCGv_i64 lo, int reg)
+{
+    if (reg != 0) {
+        tcg_gen_mov_i64(cpu_mmr[reg], hi);
+        tcg_gen_mov_i64(cpu_gpr[reg], lo);
+    }
+}
+#endif /* defined(TARGET_MIPS64) */
+
 /* Moves to/from shadow registers. */
 static inline void gen_load_srsgpr (int from, int to)
 {
@@ -27463,7 +27474,40 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
 
 static void gen_mmi_lq(CPUMIPSState *env, DisasContext *ctx)
 {
-    generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_LQ */
+#if !defined(TARGET_MIPS64)
+    generate_exception_end(ctx, EXCP_RI);
+#else
+    int base = extract32(ctx->opcode, 21, 5);
+    int rt = extract32(ctx->opcode, 16, 5);
+    int offset = sextract32(ctx->opcode, 0, 16);
+
+    TCGv_i64 addr = tcg_temp_new_i64();
+    TCGv_i64 val0 = tcg_temp_new_i64();
+    TCGv_i64 val1 = tcg_temp_new_i64();
+
+    gen_base_offset_addr(ctx, addr, base, offset);
+
+    /*
+     * The least significant four bits of the effective address are
+     * masked to zero, effectively creating an aligned address. No
+     * address exceptions due to alignment are possible.
+     */
+    tcg_gen_andi_i64(addr, addr, ~0xFULL);
+
+    tcg_gen_qemu_ld_i64(val0, addr, ctx->mem_idx, MO_UNALN | MO_64);
+    tcg_gen_addi_i64(addr, addr, 8);
+    tcg_gen_qemu_ld_i64(val1, addr, ctx->mem_idx, MO_UNALN | MO_64);
+
+#if defined(TARGET_WORDS_BIGENDIAN)
+    gen_store_mmr(val0, val1, rt);
+#else
+    gen_store_mmr(val1, val0, rt);
+#endif
+
+    tcg_temp_free_i64(val1);
+    tcg_temp_free_i64(val0);
+    tcg_temp_free_i64(addr);
+#endif /* defined(TARGET_MIPS64) */
 }
 
 static void gen_mmi_sq(DisasContext *ctx, int base, int rt, int offset)
-- 
2.19.2

[Qemu-devel] [PATCH 6/9] target/mips: Support the R5900 SQ multimedia instruction

[Fredrik Noring]

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 79505bb6c2..1c7c649d36 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2628,6 +2628,17 @@ static inline void gen_store_gpr (TCGv t, int reg)
 
 #if defined(TARGET_MIPS64)
 /* 128-bit multimedia register moves. */
+static inline void gen_load_mmr(TCGv_i64 hi, TCGv_i64 lo, int reg)
+{
+    if (reg == 0) {
+        tcg_gen_movi_i64(hi, 0);
+        tcg_gen_movi_i64(lo, 0);
+    } else {
+        tcg_gen_mov_i64(hi, cpu_mmr[reg]);
+        tcg_gen_mov_i64(lo, cpu_gpr[reg]);
+    }
+}
+
 static inline void gen_store_mmr(TCGv_i64 hi, TCGv_i64 lo, int reg)
 {
     if (reg != 0) {
@@ -27512,7 +27523,36 @@ static void gen_mmi_lq(CPUMIPSState *env, DisasContext *ctx)
 
 static void gen_mmi_sq(DisasContext *ctx, int base, int rt, int offset)
 {
-    generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_SQ */
+#if !defined(TARGET_MIPS64)
+    generate_exception_end(ctx, EXCP_RI);
+#else
+    TCGv_i64 addr = tcg_temp_new_i64();
+    TCGv_i64 val0 = tcg_temp_new_i64();
+    TCGv_i64 val1 = tcg_temp_new_i64();
+
+    gen_base_offset_addr(ctx, addr, base, offset);
+
+    /*
+     * The least significant four bits of the effective address are
+     * masked to zero, effectively creating an aligned address. No
+     * address exceptions due to alignment are possible.
+     */
+    tcg_gen_andi_i64(addr, addr, ~0xFULL);
+
+#if defined(TARGET_WORDS_BIGENDIAN)
+    gen_load_mmr(val0, val1, rt);
+#else
+    gen_load_mmr(val1, val0, rt);
+#endif
+
+    tcg_gen_qemu_st_i64(val0, addr, ctx->mem_idx, MO_UNALN | MO_64);
+    tcg_gen_addi_i64(addr, addr, 8);
+    tcg_gen_qemu_st_i64(val1, addr, ctx->mem_idx, MO_UNALN | MO_64);
+
+    tcg_temp_free_i64(val1);
+    tcg_temp_free_i64(val0);
+    tcg_temp_free_i64(addr);
+#endif /* defined(TARGET_MIPS64) */
 }
 
 /*
@@ -27540,7 +27580,7 @@ static void decode_mmi_sq(CPUMIPSState *env, DisasContext *ctx)
 {
     int base = extract32(ctx->opcode, 21, 5);
     int rt = extract32(ctx->opcode, 16, 5);
-    int offset = extract32(ctx->opcode, 0, 16);
+    int offset = sextract32(ctx->opcode, 0, 16);
 
 #ifdef CONFIG_USER_ONLY
     uint32_t op1 = MASK_SPECIAL3(ctx->opcode);
-- 
2.19.2

[Qemu-devel] [PATCH 7/9] tests/tcg/mips: Test R5900 multimedia instructions PCPYUD and PCPYLD

[Fredrik Noring]

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

diff --git a/tests/tcg/mips/mipsn32r5900/Makefile b/tests/tcg/mips/mipsn32r5900/Makefile
new file mode 100644
index 0000000000..f4887678ae
--- /dev/null
+++ b/tests/tcg/mips/mipsn32r5900/Makefile
@@ -0,0 +1,25 @@
+-include ../../config-host.mak
+
+CROSS=mips64r5900el-unknown-linux-gnu-
+
+SIM=qemu-mipsn32el
+SIM_FLAGS=-cpu R5900
+
+CC      = $(CROSS)gcc
+CFLAGS  = -Wall -mabi=n32 -march=r5900 -static
+
+TESTCASES = pcpyuld.tst
+
+all: $(TESTCASES)
+
+%.tst: %.c
+	$(CC) $(CFLAGS) $< -o $@
+
+check: $(TESTCASES)
+	@for case in $(TESTCASES); do \
+        echo $(SIM) $(SIM_FLAGS) ./$$case;\
+        $(SIM) $(SIM_FLAGS) ./$$case; \
+	done
+
+clean:
+	$(RM) -rf $(TESTCASES)
diff --git a/tests/tcg/mips/mipsn32r5900/pcpyuld.c b/tests/tcg/mips/mipsn32r5900/pcpyuld.c
new file mode 100644
index 0000000000..c92af6330b
--- /dev/null
+++ b/tests/tcg/mips/mipsn32r5900/pcpyuld.c
@@ -0,0 +1,46 @@
+/*
+ * Test PCPYUD and PCPYLD.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <assert.h>
+
+/* 128-bit multimedia register */
+struct mmr { uint64_t hi, lo; } __attribute__((aligned(16)));
+
+static void verify_zero(void)
+{
+    __asm__ __volatile__ (
+        "    pcpyud  $0, $0, $0\n"
+        "    pcpyld  $0, $0, $0\n"
+    );
+}
+
+static void verify_copy(void)
+{
+    const struct mmr value = {
+        .hi = 0x6665646362613938,
+        .lo = 0x3736353433323130
+    };
+    struct mmr result = { };
+
+    __asm__ __volatile__ (
+        "    pcpyld  %0, %2, %3\n"
+        "    move    %1, %0\n"
+        "    pcpyud  %0, %0, %0\n"
+        : "=r" (result.hi), "=r" (result.lo)
+        : "r" (value.hi), "r" (value.lo));
+
+    assert(value.hi == result.hi);
+    assert(value.lo == result.lo);
+}
+
+int main()
+{
+    verify_zero();
+    verify_copy();
+
+    return 0;
+}
-- 
2.19.2

[Qemu-devel] [PATCH 8/9] tests/tcg/mips: Test R5900 multimedia instruction LQ

[Fredrik Noring]

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 tests/tcg/mips/mipsn32r5900/Makefile |   3 +-
 tests/tcg/mips/mipsn32r5900/lq.c     | 111 +++++++++++++++++++++++++++
 2 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/mips/mipsn32r5900/lq.c

diff --git a/tests/tcg/mips/mipsn32r5900/Makefile b/tests/tcg/mips/mipsn32r5900/Makefile
index f4887678ae..cb3ef9d26a 100644
--- a/tests/tcg/mips/mipsn32r5900/Makefile
+++ b/tests/tcg/mips/mipsn32r5900/Makefile
@@ -8,7 +8,8 @@ SIM_FLAGS=-cpu R5900
 CC      = $(CROSS)gcc
 CFLAGS  = -Wall -mabi=n32 -march=r5900 -static
 
-TESTCASES = pcpyuld.tst
+TESTCASES = lq.tst
+TESTCASES += pcpyuld.tst
 
 all: $(TESTCASES)
 
diff --git a/tests/tcg/mips/mipsn32r5900/lq.c b/tests/tcg/mips/mipsn32r5900/lq.c
new file mode 100644
index 0000000000..5a16b12450
--- /dev/null
+++ b/tests/tcg/mips/mipsn32r5900/lq.c
@@ -0,0 +1,111 @@
+/*
+ * Test LQ.
+ */
+
+#include <stdio.h>
+#include <inttypes.h>
+#include <assert.h>
+
+/* 128-bit multimedia register */
+struct mmr { uint64_t hi, lo; } __attribute__((aligned(16)));
+
+#define LQ(base, offset) \
+    ({ \
+        uint64_t hi, lo; \
+    \
+        __asm__ __volatile__ ( \
+            "    pcpyld  %1, %1, %1\n" \
+            "    lq %1, %3(%2)\n" \
+            "    pcpyud  %0, %1, %1\n" \
+            : "=r" (hi), "=r" (lo) \
+            : "r" (base), "i" (offset)); \
+    \
+        (struct mmr) { .hi = hi, .lo = lo }; \
+    })
+
+static uint64_t ld_reference(const void *base, int16_t offset)
+{
+    const uint8_t *p = base;
+    uint64_t r = 0;
+    int i;
+
+    for (i = 0; i < 8; i++) {
+        r |= (uint64_t)p[offset + i] << (8 * i);
+    }
+
+    return r;
+}
+
+static struct mmr lq_reference(const void *base, int16_t offset)
+{
+    /*
+     * The least significant four bits of the effective address are
+     * masked to zero, effectively creating an aligned address. No
+     * address exceptions due to alignment are possible.
+     */
+    const uint8_t *b = base;
+    const uint8_t *o = &b[offset];
+    const void *a = (const void*)((unsigned long)o & ~0xFUL);
+
+    return (struct mmr) {
+        .hi = ld_reference(a, 8),
+        .lo = ld_reference(a, 0)
+    };
+}
+
+static void assert_equal_mmr(struct mmr a, struct mmr b)
+{
+    assert(a.hi == b.hi);
+    assert(a.lo == b.lo);
+}
+
+#define VERIFY_LQ(base, offset) \
+    assert_equal_mmr(LQ(base, offset), lq_reference(base, offset))
+
+int main()
+{
+    static const char data[] __attribute__((aligned(16)))=
+        "0123456789abcdef"
+        "ghijklmnopqrstuv"
+        "wxyzABCDEFGHIJKL"
+        "MNOPQRSTUVWXYZ.,";
+    int i;
+
+    for (i = 16; i < 48; i++) {
+        VERIFY_LQ(&data[i], -16);
+        VERIFY_LQ(&data[i], -15);
+        VERIFY_LQ(&data[i], -14);
+        VERIFY_LQ(&data[i], -13);
+        VERIFY_LQ(&data[i], -12);
+        VERIFY_LQ(&data[i], -11);
+        VERIFY_LQ(&data[i], -10);
+        VERIFY_LQ(&data[i], -9);
+        VERIFY_LQ(&data[i], -8);
+        VERIFY_LQ(&data[i], -7);
+        VERIFY_LQ(&data[i], -6);
+        VERIFY_LQ(&data[i], -5);
+        VERIFY_LQ(&data[i], -4);
+        VERIFY_LQ(&data[i], -3);
+        VERIFY_LQ(&data[i], -2);
+        VERIFY_LQ(&data[i], -1);
+        VERIFY_LQ(&data[i],  0);
+        VERIFY_LQ(&data[i],  1);
+        VERIFY_LQ(&data[i],  2);
+        VERIFY_LQ(&data[i],  3);
+        VERIFY_LQ(&data[i],  4);
+        VERIFY_LQ(&data[i],  5);
+        VERIFY_LQ(&data[i],  6);
+        VERIFY_LQ(&data[i],  7);
+        VERIFY_LQ(&data[i],  8);
+        VERIFY_LQ(&data[i],  9);
+        VERIFY_LQ(&data[i],  10);
+        VERIFY_LQ(&data[i],  11);
+        VERIFY_LQ(&data[i],  12);
+        VERIFY_LQ(&data[i],  13);
+        VERIFY_LQ(&data[i],  14);
+        VERIFY_LQ(&data[i],  15);
+        VERIFY_LQ(&data[i],  16);
+    }
+
+    return 0;
+}
-- 
2.19.2

[Qemu-devel] [PATCH 9/9] tests/tcg/mips: Test R5900 multimedia instruction SQ

[Fredrik Noring]

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

diff --git a/tests/tcg/mips/mipsn32r5900/Makefile b/tests/tcg/mips/mipsn32r5900/Makefile
index cb3ef9d26a..a9e9481457 100644
--- a/tests/tcg/mips/mipsn32r5900/Makefile
+++ b/tests/tcg/mips/mipsn32r5900/Makefile
@@ -10,6 +10,7 @@ CFLAGS  = -Wall -mabi=n32 -march=r5900 -static
 
 TESTCASES = lq.tst
 TESTCASES += pcpyuld.tst
+TESTCASES += sq.tst
 
 all: $(TESTCASES)
 
diff --git a/tests/tcg/mips/mipsn32r5900/sq.c b/tests/tcg/mips/mipsn32r5900/sq.c
new file mode 100644
index 0000000000..97b21711ba
--- /dev/null
+++ b/tests/tcg/mips/mipsn32r5900/sq.c
@@ -0,0 +1,105 @@
+/*
+ * Test SQ.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <assert.h>
+
+#define GUARD_BYTE 0xA9
+
+/* 128-bit multimedia register */
+struct mmr { uint64_t hi, lo; } __attribute__((aligned(16)));
+
+static uint8_t data[64];
+
+#define SQ(value, base, offset) \
+        __asm__ __volatile__ ( \
+            "    pcpyld  %0, %0, %1\n" \
+            "    sq %0, %3(%2)\n" \
+            : \
+            : "r" (value.hi), "r" (value.lo), \
+              "r" (base), "i" (offset))
+
+static void verify_sq(struct mmr value, const void *base, int16_t offset)
+{
+    /*
+     * The least significant four bits of the effective address are
+     * masked to zero, effectively creating an aligned address. No
+     * address exceptions due to alignment are possible.
+     */
+    const uint8_t *b = base;
+    const uint8_t *o = &b[offset];
+    const uint8_t *a = (const uint8_t*)((unsigned long)o & ~0xFUL);
+    size_t i;
+
+    for (i = 0; i < sizeof(data); i++) {
+        ssize_t d = &data[i] - &a[0];
+
+        if (d < 0) {
+            assert(data[i] == GUARD_BYTE);
+        } else if (d < 8) {
+            assert(data[i] == ((value.lo >> (8 * (d - 0))) & 0xFF));
+        } else if (d < 16) {
+            assert(data[i] == ((value.hi >> (8 * (d - 8))) & 0xFF));
+        } else {
+            assert(data[i] == GUARD_BYTE);
+        }
+    }
+}
+
+#define VERIFY_SQ(base, offset) \
+    do { \
+        struct mmr value = { \
+            .hi = 0x6665646362613938, \
+            .lo = 0x3736353433323130 \
+        }; \
+    \
+        memset(data, GUARD_BYTE, sizeof(data)); \
+        SQ(value, base, offset); \
+        verify_sq(value, base, offset); \
+    } while (0)
+
+int main()
+{
+    int i;
+
+    for (i = 16; i < 48; i++) {
+        VERIFY_SQ(&data[i], -16);
+        VERIFY_SQ(&data[i], -15);
+        VERIFY_SQ(&data[i], -14);
+        VERIFY_SQ(&data[i], -13);
+        VERIFY_SQ(&data[i], -12);
+        VERIFY_SQ(&data[i], -11);
+        VERIFY_SQ(&data[i], -10);
+        VERIFY_SQ(&data[i], -9);
+        VERIFY_SQ(&data[i], -8);
+        VERIFY_SQ(&data[i], -7);
+        VERIFY_SQ(&data[i], -6);
+        VERIFY_SQ(&data[i], -5);
+        VERIFY_SQ(&data[i], -4);
+        VERIFY_SQ(&data[i], -3);
+        VERIFY_SQ(&data[i], -2);
+        VERIFY_SQ(&data[i], -1);
+        VERIFY_SQ(&data[i],  0);
+        VERIFY_SQ(&data[i],  1);
+        VERIFY_SQ(&data[i],  2);
+        VERIFY_SQ(&data[i],  3);
+        VERIFY_SQ(&data[i],  4);
+        VERIFY_SQ(&data[i],  5);
+        VERIFY_SQ(&data[i],  6);
+        VERIFY_SQ(&data[i],  7);
+        VERIFY_SQ(&data[i],  8);
+        VERIFY_SQ(&data[i],  9);
+        VERIFY_SQ(&data[i],  10);
+        VERIFY_SQ(&data[i],  11);
+        VERIFY_SQ(&data[i],  12);
+        VERIFY_SQ(&data[i],  13);
+        VERIFY_SQ(&data[i],  14);
+        VERIFY_SQ(&data[i],  15);
+        VERIFY_SQ(&data[i],  16);
+    }
+
+    return 0;
+}
-- 
2.19.2

Re: [Qemu-devel] [PATCH 0/9] target/mips: Limited support for R5900 multimedia instructions

[no-reply]

Patchew URL: https://patchew.org/QEMU/cover.1547403692.git.noring@nocrew.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: cover.1547403692.git.noring@nocrew.org
Subject: [Qemu-devel] [PATCH 0/9] target/mips: Limited support for R5900 multimedia instructions
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
de6a446 tests/tcg/mips: Test R5900 multimedia instruction SQ
0088bfa tests/tcg/mips: Test R5900 multimedia instruction LQ
6ff0c3e tests/tcg/mips: Test R5900 multimedia instructions PCPYUD and PCPYLD
9339214 target/mips: Support the R5900 SQ multimedia instruction
fa1b66f target/mips: Support the R5900 LQ multimedia instruction
4d02b79 target/mips: Support the R5900 PCPYUD multimedia instruction
5f97931 target/mips: Support the R5900 PCPYLD multimedia instruction
7788cd9 target/mips: Introduce 32 R5900 128-bit multimedia registers
bfb6880 target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

=== OUTPUT BEGIN ===
1/9 Checking commit bfb6880b6968 (target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions)
2/9 Checking commit 7788cd938432 (target/mips: Introduce 32 R5900 128-bit multimedia registers)
3/9 Checking commit 5f97931d64fa (target/mips: Support the R5900 PCPYLD multimedia instruction)
4/9 Checking commit 4d02b79ee4cc (target/mips: Support the R5900 PCPYUD multimedia instruction)
5/9 Checking commit fa1b66faf41f (target/mips: Support the R5900 LQ multimedia instruction)
6/9 Checking commit 933921419413 (target/mips: Support the R5900 SQ multimedia instruction)
7/9 Checking commit 6ff0c3e10ea7 (tests/tcg/mips: Test R5900 multimedia instructions PCPYUD and PCPYLD)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#11: 
new file mode 100644

total: 0 errors, 1 warnings, 71 lines checked

Patch 7/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/9 Checking commit 0088bfa895fb (tests/tcg/mips: Test R5900 multimedia instruction LQ)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

ERROR: "(foo*)" should be "(foo *)"
#77: FILE: tests/tcg/mips/mipsn32r5900/lq.c:48:
+    const void *a = (const void*)((unsigned long)o & ~0xFUL);

ERROR: spaces required around that '=' (ctx:VxE)
#96: FILE: tests/tcg/mips/mipsn32r5900/lq.c:67:
+    static const char data[] __attribute__((aligned(16)))=
                                                          ^

total: 2 errors, 1 warnings, 120 lines checked

Patch 8/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

9/9 Checking commit de6a44645c76 (tests/tcg/mips: Test R5900 multimedia instruction SQ)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#23: 
new file mode 100644

ERROR: "(foo*)" should be "(foo *)"
#61: FILE: tests/tcg/mips/mipsn32r5900/sq.c:34:
+    const uint8_t *a = (const uint8_t*)((unsigned long)o & ~0xFUL);

total: 1 errors, 1 warnings, 112 lines checked

Patch 9/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/cover.1547403692.git.noring@nocrew.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:02 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
> 
> The R5900 MMIs operate on 128-bit registers that will be split into
> two halves: lower 64-bit GPRs and upper 64-bit MMRs. The MMIs will
> therefore be left unimplemented in 32-bit mode with the o32 ABI.
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
>  target/mips/translate.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>

Sorry, I have to disagree with this. Processor model must stay the same, and
Linux ABI should not affect, for example, the number and size of processor
registers - just like it is the case in reality.

I recommend to you to accept the mindset that QEMU is not a part of some
toolchain in a wide sense of this word.

QEMU is an independent software tool - it is for example, a compiler-agnostic
tool, and the only connection between a compiler and QEMU is the processor
documentation - and this is the reason they work together so well. They shouldn't
be "tweaked" and "integrated" to work together - both QEMU and compiler should
rely only on the processor specification, and should not know anything about the
other side.

Similar reason can be applied to ABI, QEMU Linux user mode ABI obviously
deals with specifics of ABIs, but should not touch processor model. Processor
model should not depend on conventions of any ABI. The fact that, for example,
some ABI "promises" that it will not use, let's say, some registers or instructions
is irrelevant to QEMU.

My advice for you is to focus on n32 at the time being.

o32 can be implemented with the same 64-bit processor model, but in a much
different way that you attempted some time ago. To avoid waste of our energy
and time, I am suggesting that we finish n32, and think of o32 in future.

Thanks, and hope you understand my points, and please accept my advices.

Aleksandar
 
> 
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 057aaf9a44..a538351032 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -27218,6 +27218,7 @@ static void decode_opc_special3_legacy(CPUMIPSState *env, > DisasContext *ctx)
>      }
>  }
> 
> +#if defined(TARGET_MIPS64)
>  static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
>  {
>      uint32_t opc = MASK_MMI0(ctx->opcode);
> @@ -27351,6 +27352,7 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
>          break;
>      }
>  }
> +#endif /* defined(TARGET_MIPS64) */
> 
>  static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>  {
> @@ -27360,18 +27362,6 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>      int rd = extract32(ctx->opcode, 11, 5);
> 
>      switch (opc) {
> -    case MMI_OPC_CLASS_MMI0:
> -        decode_mmi0(env, ctx);
> -        break;
> -    case MMI_OPC_CLASS_MMI1:
> -        decode_mmi1(env, ctx);
> -        break;
> -    case MMI_OPC_CLASS_MMI2:
> -        decode_mmi2(env, ctx);
> -        break;
> -    case MMI_OPC_CLASS_MMI3:
> -        decode_mmi3(env, ctx);
> -        break;
>      case MMI_OPC_MULT1:
>      case MMI_OPC_MULTU1:
>      case MMI_OPC_MADD:
> @@ -27392,6 +27382,7 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>      case MMI_OPC_MFHI1:
>          gen_HILO1_tx79(ctx, opc, rd);
>          break;
> +#if defined(TARGET_MIPS64)
>      case MMI_OPC_PLZCW:         /* TODO: MMI_OPC_PLZCW */
>      case MMI_OPC_PMFHL:         /* TODO: MMI_OPC_PMFHL */
>      case MMI_OPC_PMTHL:         /* TODO: MMI_OPC_PMTHL */
> @@ -27403,6 +27394,19 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>      case MMI_OPC_PSRAW:         /* TODO: MMI_OPC_PSRAW */
>          generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_CLASS_MMI */
>          break;
> +    case MMI_OPC_CLASS_MMI0:
> +        decode_mmi0(env, ctx);
> +        break;
> +    case MMI_OPC_CLASS_MMI1:
> +        decode_mmi1(env, ctx);
> +        break;
> +    case MMI_OPC_CLASS_MMI2:
> +        decode_mmi2(env, ctx);
> +        break;
> +    case MMI_OPC_CLASS_MMI3:
> +        decode_mmi3(env, ctx);
> +        break;
> +#endif
>      default:
>          MIPS_INVAL("TX79 MMI class");
>          generate_exception_end(ctx, EXCP_RI);
> --
> 2.19.2

Re: [Qemu-devel] [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia registers

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:03 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia registers
> 
> The 32 R5900 128-bit MMRs are split into two 64-bit halves: the lower
> halves are the GPRs and the upper halves are accessible by the R5900-
> specific multimedia instructions.
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
>  target/mips/cpu.h       |  2 ++
>  target/mips/translate.c | 14 ++++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 03c03fd8c6..9ff4a68d90 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -180,6 +180,8 @@ struct TCState {
>  #define MXU_CR_RD_EN    1
>  #define MXU_CR_MXU_EN   0
> 
> +    /* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
> +    uint64_t mmr[32];
>  };
> 
>  typedef struct CPUMIPSState CPUMIPSState;
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index a538351032..9d5150ec8b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -2455,7 +2455,10 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
>  static TCGv_i64 fpu_f64[32];
>  static TCGv_i64 msa_wr_d[64];
> 
> -#if !defined(TARGET_MIPS64)
> +#if defined(TARGET_MIPS64)
> +/* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
> +static TCGv_i64 cpu_mmr[32];
> +#else

Naming is perfect, but:

#if-#else-#endif is confusing here:
- MMR declaration should be under it own #if defined(TARGET_MIPS64)
- MXU declaration should be under separate #if !defined(TARGET_MIPS64)
(we are not dealing with two variants (64/32) of the same item, but with two
separate items, one is valid for 64-bit only, another for 32-bit only)

>  /* MXU registers */
>  static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
>  static TCGv mxu_CR;
> @@ -29785,7 +29788,14 @@ void mips_tcg_init(void)
>      fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
>                                         offsetof(CPUMIPSState, active_fpu.fcr31),
>                                         "fcr31");
> -#if !defined(TARGET_MIPS64)
> +#if defined(TARGET_MIPS64)
> +    cpu_mmr[0] = NULL;
> +    for (i = 1; i < 32; i++)
> +        cpu_mmr[i] = tcg_global_mem_new_i64(cpu_env,
> +                                            offsetof(CPUMIPSState,
> +                                                     active_tc.mmr[i]),
> +                                            regnames[i]);
> +#else
>      for (i = 0; i < NUMBER_OF_MXU_REGISTERS - 1; i++) {
>          mxu_gpr[i] = tcg_global_mem_new(cpu_env,
>                                          offsetof(CPUMIPSState,
> --

The same applies here.

> 2.19.2

Re: [Qemu-devel] [PATCH 3/9] target/mips: Support the R5900 PCPYLD multimedia instruction

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:03 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 3/9] target/mips: Support the R5900 PCPYLD multimedia instruction
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
>  target/mips/translate.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 9d5150ec8b..40faf9cb36 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -27293,11 +27293,34 @@ static void decode_mmi1(CPUMIPSState *env, DisasContext *ctx)
>      }
>  }
> 
> +static void gen_mmi_pcpyld(DisasContext *ctx, int rd, int rs, int rt)
> +{
> +    if (rd != 0) {
> +        if (rs != 0) {
> +            tcg_gen_mov_i64(cpu_mmr[rd], cpu_gpr[rs]);
> +        } else {
> +            tcg_gen_movi_i64(cpu_mmr[rd], 0);
> +        }
> +
> +        if (rt != 0) {
> +            tcg_gen_mov_i64(cpu_gpr[rd], cpu_gpr[rt]);
> +        } else {
> +            tcg_gen_movi_i64(cpu_gpr[rd], 0);
> +        }
> +    }
> +}
> +
>  static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
>  {
>      uint32_t opc = MASK_MMI2(ctx->opcode);
> +    int rs = extract32(ctx->opcode, 21, 5);
> +    int rt = extract32(ctx->opcode, 16, 5);
> +    int rd = extract32(ctx->opcode, 11, 5);
> 

If you move last three lines to gen_mmi_pcpyld(), and correct its arguments,
this patch has:

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

>      switch (opc) {
> +    case MMI_OPC_2_PCPYLD:
> +        gen_mmi_pcpyld(ctx, rd, rs, rt);
> +        break;
>      case MMI_OPC_2_PMADDW:    /* TODO: MMI_OPC_2_PMADDW */
>      case MMI_OPC_2_PSLLVW:    /* TODO: MMI_OPC_2_PSLLVW */
>      case MMI_OPC_2_PSRLVW:    /* TODO: MMI_OPC_2_PSRLVW */
> @@ -27307,7 +27330,6 @@ static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
>      case MMI_OPC_2_PINTH:     /* TODO: MMI_OPC_2_PINTH */
>      case MMI_OPC_2_PMULTW:    /* TODO: MMI_OPC_2_PMULTW */
>      case MMI_OPC_2_PDIVW:     /* TODO: MMI_OPC_2_PDIVW */
> -    case MMI_OPC_2_PCPYLD:    /* TODO: MMI_OPC_2_PCPYLD */
>      case MMI_OPC_2_PMADDH:    /* TODO: MMI_OPC_2_PMADDH */
>      case MMI_OPC_2_PHMADH:    /* TODO: MMI_OPC_2_PHMADH */
>      case MMI_OPC_2_PAND:      /* TODO: MMI_OPC_2_PAND */
> --
> 2.19.2

Re: [Qemu-devel] [PATCH 4/9] target/mips: Support the R5900 PCPYUD multimedia instruction

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:04 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 4/9] target/mips: Support the R5900 PCPYUD multimedia instruction
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
>  target/mips/translate.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 40faf9cb36..8c350729bc 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -27351,11 +27351,34 @@ static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
>      }
>  }
> 
> +static void gen_mmi_pcpyud(DisasContext *ctx, int rd, int rs, int rt)
> +{
> +    if (rd != 0) {
> +        if (rs != 0) {
> +            tcg_gen_mov_i64(cpu_gpr[rd], cpu_mmr[rs]);
> +        } else {
> +            tcg_gen_movi_i64(cpu_gpr[rd], 0);
> +        }
> +
> +        if (rt != 0) {
> +            tcg_gen_mov_i64(cpu_mmr[rd], cpu_mmr[rt]);
> +        } else {
> +            tcg_gen_movi_i64(cpu_mmr[rd], 0);
> +        }
> +    }
> +}
> +
>  static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
>  {
>      uint32_t opc = MASK_MMI3(ctx->opcode);
> +    int rs = extract32(ctx->opcode, 21, 5);
> +    int rt = extract32(ctx->opcode, 16, 5);
> +    int rd = extract32(ctx->opcode, 11, 5);
> 

The similar caveat as in the previous patch: If you move last three lines
to gen_mmi_pcpyud(), and correct its arguments, this patch has:

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

>      switch (opc) {
> +    case MMI_OPC_3_PCPYUD:
> +        gen_mmi_pcpyud(ctx, rd, rs, rt);
> +        break;
>      case MMI_OPC_3_PMADDUW:    /* TODO: MMI_OPC_3_PMADDUW */
>      case MMI_OPC_3_PSRAVW:     /* TODO: MMI_OPC_3_PSRAVW */
>      case MMI_OPC_3_PMTHI:      /* TODO: MMI_OPC_3_PMTHI */
> @@ -27363,7 +27386,6 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
>      case MMI_OPC_3_PINTEH:     /* TODO: MMI_OPC_3_PINTEH */
>      case MMI_OPC_3_PMULTUW:    /* TODO: MMI_OPC_3_PMULTUW */
>      case MMI_OPC_3_PDIVUW:     /* TODO: MMI_OPC_3_PDIVUW */
> -    case MMI_OPC_3_PCPYUD:     /* TODO: MMI_OPC_3_PCPYUD */
>      case MMI_OPC_3_POR:        /* TODO: MMI_OPC_3_POR */
>      case MMI_OPC_3_PNOR:       /* TODO: MMI_OPC_3_PNOR */
>      case MMI_OPC_3_PEXCH:      /* TODO: MMI_OPC_3_PEXCH */
> --
> 2.19.2

Re: [Qemu-devel] [PATCH 5/9] target/mips: Support the R5900 LQ multimedia instruction

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:05 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 5/9] target/mips: Support the R5900 LQ multimedia instruction
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>

Please provide a commit message, even if it is the same as the patch title.

> ---
>  target/mips/translate.c | 46 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 8c350729bc..79505bb6c2 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -2626,6 +2626,17 @@ static inline void gen_store_gpr (TCGv t, int reg)
>          tcg_gen_mov_tl(cpu_gpr[reg], t);
>  }
> 
> +#if defined(TARGET_MIPS64)
> +/* 128-bit multimedia register moves. */
> +static inline void gen_store_mmr(TCGv_i64 hi, TCGv_i64 lo, int reg)
> +{
> +    if (reg != 0) {
> +        tcg_gen_mov_i64(cpu_mmr[reg], hi);
> +        tcg_gen_mov_i64(cpu_gpr[reg], lo);
> +    }
> +}
> +#endif /* defined(TARGET_MIPS64) */
> +
>  /* Moves to/from shadow registers. */
>  static inline void gen_load_srsgpr (int from, int to)
>  {
> @@ -27463,7 +27474,40 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
> 
>  static void gen_mmi_lq(CPUMIPSState *env, DisasContext *ctx)
>  {
> -    generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_LQ */
> +#if !defined(TARGET_MIPS64)
> +    generate_exception_end(ctx, EXCP_RI);
> +#else

TARGET_MIPS64 should always be defined for R5900, IMHO.

> +    int base = extract32(ctx->opcode, 21, 5);
> +    int rt = extract32(ctx->opcode, 16, 5);
> +    int offset = sextract32(ctx->opcode, 0, 16);
> +
> +    TCGv_i64 addr = tcg_temp_new_i64();
> +    TCGv_i64 val0 = tcg_temp_new_i64();
> +    TCGv_i64 val1 = tcg_temp_new_i64();
> +
> +    gen_base_offset_addr(ctx, addr, base, offset);
> +
> +    /*
> +     * The least significant four bits of the effective address are
> +     * masked to zero, effectively creating an aligned address. No
> +     * address exceptions due to alignment are possible.
> +     */
> +    tcg_gen_andi_i64(addr, addr, ~0xFULL);
> +
> +    tcg_gen_qemu_ld_i64(val0, addr, ctx->mem_idx, MO_UNALN | MO_64);
> +    tcg_gen_addi_i64(addr, addr, 8);
> +    tcg_gen_qemu_ld_i64(val1, addr, ctx->mem_idx, MO_UNALN | MO_64);
> +
> +#if defined(TARGET_WORDS_BIGENDIAN)
> +    gen_store_mmr(val0, val1, rt);
> +#else
> +    gen_store_mmr(val1, val0, rt);
> +#endif
> +
> +    tcg_temp_free_i64(val1);
> +    tcg_temp_free_i64(val0);
> +    tcg_temp_free_i64(addr);
> +#endif /* defined(TARGET_MIPS64) */
>  }
> 
>  static void gen_mmi_sq(DisasContext *ctx, int base, int rt, int offset)
> --
> 2.19.2

Re: [Qemu-devel] [PATCH 6/9] target/mips: Support the R5900 SQ multimedia instruction

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:06 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 6/9] target/mips: Support the R5900 SQ multimedia instruction
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---
>  target/mips/translate.c | 44 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 79505bb6c2..1c7c649d36 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -2628,6 +2628,17 @@ static inline void gen_store_gpr (TCGv t, int reg)
> 
>  #if defined(TARGET_MIPS64)
>  /* 128-bit multimedia register moves. */
> +static inline void gen_load_mmr(TCGv_i64 hi, TCGv_i64 lo, int reg)
> +{
> +    if (reg == 0) {
> +        tcg_gen_movi_i64(hi, 0);
> +        tcg_gen_movi_i64(lo, 0);
> +    } else {
> +        tcg_gen_mov_i64(hi, cpu_mmr[reg]);
> +        tcg_gen_mov_i64(lo, cpu_gpr[reg]);
> +    }
> +}
> +
>  static inline void gen_store_mmr(TCGv_i64 hi, TCGv_i64 lo, int reg)
>  {
>      if (reg != 0) {
> @@ -27512,7 +27523,36 @@ static void gen_mmi_lq(CPUMIPSState *env, DisasContext *ctx)
> 
>  static void gen_mmi_sq(DisasContext *ctx, int base, int rt, int offset)
>  {
> -    generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_OPC_SQ */
> +#if !defined(TARGET_MIPS64)
> +    generate_exception_end(ctx, EXCP_RI);
> +#else

The same objection as in the previous patch.

> +    TCGv_i64 addr = tcg_temp_new_i64();
> +    TCGv_i64 val0 = tcg_temp_new_i64();
> +    TCGv_i64 val1 = tcg_temp_new_i64();
> +
> +    gen_base_offset_addr(ctx, addr, base, offset);
> +
> +    /*
> +     * The least significant four bits of the effective address are
> +     * masked to zero, effectively creating an aligned address. No
> +     * address exceptions due to alignment are possible.
> +     */
> +    tcg_gen_andi_i64(addr, addr, ~0xFULL);
> +
> +#if defined(TARGET_WORDS_BIGENDIAN)
> +    gen_load_mmr(val0, val1, rt);
> +#else
> +    gen_load_mmr(val1, val0, rt);
> +#endif
> +
> +    tcg_gen_qemu_st_i64(val0, addr, ctx->mem_idx, MO_UNALN | MO_64);
> +    tcg_gen_addi_i64(addr, addr, 8);
> +    tcg_gen_qemu_st_i64(val1, addr, ctx->mem_idx, MO_UNALN | MO_64);
> +
> +    tcg_temp_free_i64(val1);
> +    tcg_temp_free_i64(val0);
> +    tcg_temp_free_i64(addr);
> +#endif /* defined(TARGET_MIPS64) */
>  }
> 
>  /*
> @@ -27540,7 +27580,7 @@ static void decode_mmi_sq(CPUMIPSState *env, DisasContext *ctx)
>  {
>      int base = extract32(ctx->opcode, 21, 5);
>      int rt = extract32(ctx->opcode, 16, 5);
> -    int offset = extract32(ctx->opcode, 0, 16);
> +    int offset = sextract32(ctx->opcode, 0, 16);
> 
>  #ifdef CONFIG_USER_ONLY
>      uint32_t op1 = MASK_SPECIAL3(ctx->opcode);
> --
> 2.19.2

Re: [Qemu-devel] [PATCH 7/9] tests/tcg/mips: Test R5900 multimedia instructions PCPYUD and PCPYLD

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:07 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 7/9] tests/tcg/mips: Test R5900 multimedia instructions PCPYUD and PCPYLD
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---

Please just add a commit message. Otherwise:

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

>  tests/tcg/mips/mipsn32r5900/Makefile  | 25 +++++++++++++++
>  tests/tcg/mips/mipsn32r5900/pcpyuld.c | 46 +++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+)
>  create mode 100644 tests/tcg/mips/mipsn32r5900/Makefile
>  create mode 100644 tests/tcg/mips/mipsn32r5900/pcpyuld.c
> 
> diff --git a/tests/tcg/mips/mipsn32r5900/Makefile b/tests/tcg/mips/mipsn32r5900/Makefile
> new file mode 100644
> index 0000000000..f4887678ae
> --- /dev/null
> +++ b/tests/tcg/mips/mipsn32r5900/Makefile
> @@ -0,0 +1,25 @@
> +-include ../../config-host.mak
> +
> +CROSS=mips64r5900el-unknown-linux-gnu-
> +
> +SIM=qemu-mipsn32el
> +SIM_FLAGS=-cpu R5900
> +
> +CC      = $(CROSS)gcc
> +CFLAGS  = -Wall -mabi=n32 -march=r5900 -static
> +
> +TESTCASES = pcpyuld.tst
> +
> +all: $(TESTCASES)
> +
> +%.tst: %.c
> +       $(CC) $(CFLAGS) $< -o $@
> +
> +check: $(TESTCASES)
> +       @for case in $(TESTCASES); do \
> +        echo $(SIM) $(SIM_FLAGS) ./$$case;\
> +        $(SIM) $(SIM_FLAGS) ./$$case; \
> +       done
> +
> +clean:
> +       $(RM) -rf $(TESTCASES)
> diff --git a/tests/tcg/mips/mipsn32r5900/pcpyuld.c b/tests/tcg/mips/mipsn32r5900/pcpyuld.c
> new file mode 100644
> index 0000000000..c92af6330b
> --- /dev/null
> +++ b/tests/tcg/mips/mipsn32r5900/pcpyuld.c
> @@ -0,0 +1,46 @@
> +/*
> + * Test PCPYUD and PCPYLD.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <assert.h>
> +
> +/* 128-bit multimedia register */
> +struct mmr { uint64_t hi, lo; } __attribute__((aligned(16)));
> +
> +static void verify_zero(void)
> +{
> +    __asm__ __volatile__ (
> +        "    pcpyud  $0, $0, $0\n"
> +        "    pcpyld  $0, $0, $0\n"
> +    );
> +}
> +
> +static void verify_copy(void)
> +{
> +    const struct mmr value = {
> +        .hi = 0x6665646362613938,
> +        .lo = 0x3736353433323130
> +    };
> +    struct mmr result = { };
> +
> +    __asm__ __volatile__ (
> +        "    pcpyld  %0, %2, %3\n"
> +        "    move    %1, %0\n"
> +        "    pcpyud  %0, %0, %0\n"
> +        : "=r" (result.hi), "=r" (result.lo)
> +        : "r" (value.hi), "r" (value.lo));
> +
> +    assert(value.hi == result.hi);
> +    assert(value.lo == result.lo);
> +}
> +
> +int main()
> +{
> +    verify_zero();
> +    verify_copy();
> +
> +    return 0;
> +}
> --
> 2.19.2
> 
> 

Re: [Qemu-devel] [PATCH 8/9] tests/tcg/mips: Test R5900 multimedia instruction LQ

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:08 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 8/9] tests/tcg/mips: Test R5900 multimedia instruction LQ
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---

The commit message is missing. I will deal with the rest of this patch once LQ
implementation patch is accepted.

>  tests/tcg/mips/mipsn32r5900/Makefile |   3 +-
>  tests/tcg/mips/mipsn32r5900/lq.c     | 111 +++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/mips/mipsn32r5900/lq.c
> 
> diff --git a/tests/tcg/mips/mipsn32r5900/Makefile b/tests/tcg/mips/mipsn32r5900/Makefile
> index f4887678ae..cb3ef9d26a 100644
> --- a/tests/tcg/mips/mipsn32r5900/Makefile
> +++ b/tests/tcg/mips/mipsn32r5900/Makefile
> @@ -8,7 +8,8 @@ SIM_FLAGS=-cpu R5900
>  CC      = $(CROSS)gcc
>  CFLAGS  = -Wall -mabi=n32 -march=r5900 -static
> 
> -TESTCASES = pcpyuld.tst
> +TESTCASES = lq.tst
> +TESTCASES += pcpyuld.tst
> 
>  all: $(TESTCASES)
> 
> diff --git a/tests/tcg/mips/mipsn32r5900/lq.c b/tests/tcg/mips/mipsn32r5900/lq.c
> new file mode 100644
> index 0000000000..5a16b12450
> --- /dev/null
> +++ b/tests/tcg/mips/mipsn32r5900/lq.c
> @@ -0,0 +1,111 @@
> +/*
> + * Test LQ.
> + */
> +
> +#include <stdio.h>
> +#include <inttypes.h>
> +#include <assert.h>
> +
> +/* 128-bit multimedia register */
> +struct mmr { uint64_t hi, lo; } __attribute__((aligned(16)));
> +
> +#define LQ(base, offset) \
> +    ({ \
> +        uint64_t hi, lo; \
> +    \
> +        __asm__ __volatile__ ( \
> +            "    pcpyld  %1, %1, %1\n" \
> +            "    lq %1, %3(%2)\n" \
> +            "    pcpyud  %0, %1, %1\n" \
> +            : "=r" (hi), "=r" (lo) \
> +            : "r" (base), "i" (offset)); \
> +    \
> +        (struct mmr) { .hi = hi, .lo = lo }; \
> +    })
> +
> +static uint64_t ld_reference(const void *base, int16_t offset)
> +{
> +    const uint8_t *p = base;
> +    uint64_t r = 0;
> +    int i;
> +
> +    for (i = 0; i < 8; i++) {
> +        r |= (uint64_t)p[offset + i] << (8 * i);
> +    }
> +
> +    return r;
> +}
> +
> +static struct mmr lq_reference(const void *base, int16_t offset)
> +{
> +    /*
> +     * The least significant four bits of the effective address are
> +     * masked to zero, effectively creating an aligned address. No
> +     * address exceptions due to alignment are possible.
> +     */
> +    const uint8_t *b = base;
> +    const uint8_t *o = &b[offset];
> +    const void *a = (const void*)((unsigned long)o & ~0xFUL);
> +
> +    return (struct mmr) {
> +        .hi = ld_reference(a, 8),
> +        .lo = ld_reference(a, 0)
> +    };
> +}
> +
> +static void assert_equal_mmr(struct mmr a, struct mmr b)
> +{
> +    assert(a.hi == b.hi);
> +    assert(a.lo == b.lo);
> +}
> +
> +#define VERIFY_LQ(base, offset) \
> +    assert_equal_mmr(LQ(base, offset), lq_reference(base, offset))
> +
> +int main()
> +{
> +    static const char data[] __attribute__((aligned(16)))=
> +        "0123456789abcdef"
> +        "ghijklmnopqrstuv"
> +        "wxyzABCDEFGHIJKL"
> +        "MNOPQRSTUVWXYZ.,";
> +    int i;
> +
> +    for (i = 16; i < 48; i++) {
> +        VERIFY_LQ(&data[i], -16);
> +        VERIFY_LQ(&data[i], -15);
> +        VERIFY_LQ(&data[i], -14);
> +        VERIFY_LQ(&data[i], -13);
> +        VERIFY_LQ(&data[i], -12);
> +        VERIFY_LQ(&data[i], -11);
> +        VERIFY_LQ(&data[i], -10);
> +        VERIFY_LQ(&data[i], -9);
> +        VERIFY_LQ(&data[i], -8);
> +        VERIFY_LQ(&data[i], -7);
> +        VERIFY_LQ(&data[i], -6);
> +        VERIFY_LQ(&data[i], -5);
> +        VERIFY_LQ(&data[i], -4);
> +        VERIFY_LQ(&data[i], -3);
> +        VERIFY_LQ(&data[i], -2);
> +        VERIFY_LQ(&data[i], -1);
> +        VERIFY_LQ(&data[i],  0);
> +        VERIFY_LQ(&data[i],  1);
> +        VERIFY_LQ(&data[i],  2);
> +        VERIFY_LQ(&data[i],  3);
> +        VERIFY_LQ(&data[i],  4);
> +        VERIFY_LQ(&data[i],  5);
> +        VERIFY_LQ(&data[i],  6);
> +        VERIFY_LQ(&data[i],  7);
> +        VERIFY_LQ(&data[i],  8);
> +        VERIFY_LQ(&data[i],  9);
> +        VERIFY_LQ(&data[i],  10);
> +        VERIFY_LQ(&data[i],  11);
> +        VERIFY_LQ(&data[i],  12);
> +        VERIFY_LQ(&data[i],  13);
> +        VERIFY_LQ(&data[i],  14);
> +        VERIFY_LQ(&data[i],  15);
> +        VERIFY_LQ(&data[i],  16);
> +    }
> +
> +    return 0;
> +}
> --
> 2.19.2
> 
> 

Re: [Qemu-devel] [PATCH 9/9] tests/tcg/mips: Test R5900 multimedia instruction SQ

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Sunday, January 13, 2019 8:09 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: [PATCH 9/9] tests/tcg/mips: Test R5900 multimedia instruction SQ
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>
> ---

The commit message is missing. I will deal with the rest of this patch once LQ
implementation patch is accepted. Otherwise an interesting testing idea.


>  tests/tcg/mips/mipsn32r5900/Makefile |   1 +
>  tests/tcg/mips/mipsn32r5900/sq.c     | 105 +++++++++++++++++++++++++++
>  2 files changed, 106 insertions(+)
>  create mode 100644 tests/tcg/mips/mipsn32r5900/sq.c
> 
> diff --git a/tests/tcg/mips/mipsn32r5900/Makefile b/tests/tcg/mips/mipsn32r5900/Makefile
> index cb3ef9d26a..a9e9481457 100644
> --- a/tests/tcg/mips/mipsn32r5900/Makefile
> +++ b/tests/tcg/mips/mipsn32r5900/Makefile
> @@ -10,6 +10,7 @@ CFLAGS  = -Wall -mabi=n32 -march=r5900 -static
> 
>  TESTCASES = lq.tst
>  TESTCASES += pcpyuld.tst
> +TESTCASES += sq.tst
> 
>  all: $(TESTCASES)
> 
> diff --git a/tests/tcg/mips/mipsn32r5900/sq.c b/tests/tcg/mips/mipsn32r5900/sq.c
> new file mode 100644
> index 0000000000..97b21711ba
> --- /dev/null
> +++ b/tests/tcg/mips/mipsn32r5900/sq.c
> @@ -0,0 +1,105 @@
> +/*
> + * Test SQ.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <assert.h>
> +
> +#define GUARD_BYTE 0xA9
> +
> +/* 128-bit multimedia register */
> +struct mmr { uint64_t hi, lo; } __attribute__((aligned(16)));
> +
> +static uint8_t data[64];
> +
> +#define SQ(value, base, offset) \
> +        __asm__ __volatile__ ( \
> +            "    pcpyld  %0, %0, %1\n" \
> +            "    sq %0, %3(%2)\n" \
> +            : \
> +            : "r" (value.hi), "r" (value.lo), \
> +              "r" (base), "i" (offset))
> +
> +static void verify_sq(struct mmr value, const void *base, int16_t offset)
> +{
> +    /*
> +     * The least significant four bits of the effective address are
> +     * masked to zero, effectively creating an aligned address. No
> +     * address exceptions due to alignment are possible.
> +     */
> +    const uint8_t *b = base;
> +    const uint8_t *o = &b[offset];
> +    const uint8_t *a = (const uint8_t*)((unsigned long)o & ~0xFUL);
> +    size_t i;
> +
> +    for (i = 0; i < sizeof(data); i++) {
> +        ssize_t d = &data[i] - &a[0];
> +
> +        if (d < 0) {
> +            assert(data[i] == GUARD_BYTE);
> +        } else if (d < 8) {
> +            assert(data[i] == ((value.lo >> (8 * (d - 0))) & 0xFF));
> +        } else if (d < 16) {
> +            assert(data[i] == ((value.hi >> (8 * (d - 8))) & 0xFF));
> +        } else {
> +            assert(data[i] == GUARD_BYTE);
> +        }
> +    }
> +}
> +
> +#define VERIFY_SQ(base, offset) \
> +    do { \
> +        struct mmr value = { \
> +            .hi = 0x6665646362613938, \
> +            .lo = 0x3736353433323130 \
> +        }; \
> +    \
> +        memset(data, GUARD_BYTE, sizeof(data)); \
> +        SQ(value, base, offset); \
> +        verify_sq(value, base, offset); \
> +    } while (0)
> +
> +int main()
> +{
> +    int i;
> +
> +    for (i = 16; i < 48; i++) {
> +        VERIFY_SQ(&data[i], -16);
> +        VERIFY_SQ(&data[i], -15);
> +        VERIFY_SQ(&data[i], -14);
> +        VERIFY_SQ(&data[i], -13);
> +        VERIFY_SQ(&data[i], -12);
> +        VERIFY_SQ(&data[i], -11);
> +        VERIFY_SQ(&data[i], -10);
> +        VERIFY_SQ(&data[i], -9);
> +        VERIFY_SQ(&data[i], -8);
> +        VERIFY_SQ(&data[i], -7);
> +        VERIFY_SQ(&data[i], -6);
> +        VERIFY_SQ(&data[i], -5);
> +        VERIFY_SQ(&data[i], -4);
> +        VERIFY_SQ(&data[i], -3);
> +        VERIFY_SQ(&data[i], -2);
> +        VERIFY_SQ(&data[i], -1);
> +        VERIFY_SQ(&data[i],  0);
> +        VERIFY_SQ(&data[i],  1);
> +        VERIFY_SQ(&data[i],  2);
> +        VERIFY_SQ(&data[i],  3);
> +        VERIFY_SQ(&data[i],  4);
> +        VERIFY_SQ(&data[i],  5);
> +        VERIFY_SQ(&data[i],  6);
> +        VERIFY_SQ(&data[i],  7);
> +        VERIFY_SQ(&data[i],  8);
> +        VERIFY_SQ(&data[i],  9);
> +        VERIFY_SQ(&data[i],  10);
> +        VERIFY_SQ(&data[i],  11);
> +        VERIFY_SQ(&data[i],  12);
> +        VERIFY_SQ(&data[i],  13);
> +        VERIFY_SQ(&data[i],  14);
> +        VERIFY_SQ(&data[i],  15);
> +        VERIFY_SQ(&data[i],  16);
> +    }
> +
> +    return 0;
> +}
> --
> 2.19.2
> 
> 

Re: [Qemu-devel] [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

[Fredrik Noring]

Hi Aleksandar,

> Sorry, I have to disagree with this.

It was, without a doubt, entirely clear that the o32 ABI was going to stay
in after this MMI patch series. In other words, this series does not imply
the removal of o32. This is obvious, as discussed previously, since the o32
ABI is currently the main use case for R5900 QEMU and the reason the R5900
was implemented for QEMU to begin with.

> Processor model must stay the same, and
> Linux ABI should not affect, for example, the number and size of processor
> registers - just like it is the case in reality.

I thought Maciej's reply to you was quite clear on this topic?

> QEMU is an independent software tool - it is for example, a compiler-agnostic
> tool, and the only connection between a compiler and QEMU is the processor
> documentation - and this is the reason they work together so well. They shouldn't
> be "tweaked" and "integrated" to work together - both QEMU and compiler should
> rely only on the processor specification, and should not know anything about the
> other side.

The approach was to implement ABIs and instructions that are actually used,
and leave unused or optional instructions for later. The reverse, removing
ABIs in-use (o32) and focusing on unused instructions (MMIs) does not make
sense, so I will obviously not do that.

> My advice for you is to focus on n32 at the time being.
> 
> o32 can be implemented with the same 64-bit processor model, but in a much
> different way that you attempted some time ago. To avoid waste of our energy
> and time, I am suggesting that we finish n32, and think of o32 in future.

The o32 ABI is more important than n32 now, because o32 is in-use and
ready with Glibc, GCC, GAS and the Linux kernel. n32 is easy to enable
with a three-line patch, so we can actually use both now.

I recommend that we implement limited support for MMIs for n32 and
eventually system mode, and leave it as unsupported for o32 for the time
being. [ Perhaps MMIs for o32 could be implemented with 96-bit registers,
in contrast to the 64-bit registers for n32, but having LW and LQ without
LD seems strange to me. ]

Fredrik

Re: [Qemu-devel] [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

[Aleksandar Markovic]

> From: Fredrik Noring <noring@nocrew.org>
> Sent: Wednesday, January 16, 2019 4:36 PM
> To: Aleksandar Markovic
> Cc: Aurelien Jarno; Philippe Mathieu-Daudé; Jürgen Urban; Maciej W. Rozycki; > qemu-devel@nongnu.org
> Subject: Re: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
> 
> Hi Aleksandar,
> 
> > Sorry, I have to disagree with this.
> 
> It was, without a doubt, entirely clear that the o32 ABI was going to stay
> in after this MMI patch series. In other words, this series does not imply
> the removal of o32. This is obvious, as discussed previously, since the o32
> ABI is currently the main use case for R5900 QEMU and the reason the R5900
> was implemented for QEMU to begin with.
> 

Fredrik,

Modeling a 64-bit processor as a 32-bit one should not be a part of QEMU
upstream.

Thanks for your efforts so far,
Aleksandar

> > Processor model must stay the same, and
> > Linux ABI should not affect, for example, the number and size of processor
> > registers - just like it is the case in reality.
> 
> I thought Maciej's reply to you was quite clear on this topic?
> 
> > QEMU is an independent software tool - it is for example, a compiler-agnostic
> > tool, and the only connection between a compiler and QEMU is the processor
> > documentation - and this is the reason they work together so well. They shouldn't
> > be "tweaked" and "integrated" to work together - both QEMU and compiler should
> > rely only on the processor specification, and should not know anything about the
> > other side.
> 
> The approach was to implement ABIs and instructions that are actually used,
> and leave unused or optional instructions for later. The reverse, removing
> ABIs in-use (o32) and focusing on unused instructions (MMIs) does not make
> sense, so I will obviously not do that.
> 
> > My advice for you is to focus on n32 at the time being.
> >
> > o32 can be implemented with the same 64-bit processor model, but in a much
> > different way that you attempted some time ago. To avoid waste of our energy
> > and time, I am suggesting that we finish n32, and think of o32 in future.
> 
> The o32 ABI is more important than n32 now, because o32 is in-use and
> ready with Glibc, GCC, GAS and the Linux kernel. n32 is easy to enable
> with a three-line patch, so we can actually use both now.
> 
> I recommend that we implement limited support for MMIs for n32 and
> eventually system mode, and leave it as unsupported for o32 for the time
> being. [ Perhaps MMIs for o32 could be implemented with 96-bit registers,
> in contrast to the 64-bit registers for n32, but having LW and LQ without
> LD seems strange to me. ]
> 
> Fredrik

Re: [Qemu-devel] [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia registers

[Aleksandar Markovic]

> From: Aleksandar Markovic
> Sent: Tuesday, January 15, 2019 10:20 PM
> To: Fredrik Noring; Aurelien Jarno; Philippe Mathieu-Daudé
> Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> Subject: Re: [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia registers
> 
> > From: Fredrik Noring <noring@nocrew.org>
> > Sent: Sunday, January 13, 2019 8:03 PM
> > To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé
> > Cc: Jürgen Urban; Maciej W. Rozycki; qemu-devel@nongnu.org
> > Subject: [PATCH 2/9] target/mips: Introduce 32 R5900 128-bit multimedia registers
> >
> > The 32 R5900 128-bit MMRs are split into two 64-bit halves: the lower
> > halves are the GPRs and the upper halves are accessible by the R5900-
> > specific multimedia instructions.
> >
> > Signed-off-by: Fredrik Noring <noring@nocrew.org>
> > ---

Since this patch is not a subject of any disagreement, and "#ifdef"
structure is of a cosmetic nature, I will do these minor cosmetic
changes while applying, and insert this patch in the next MIPS pull
request, scheduled shortly.

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

Other patches from this series will not be in the pull request.

> >  target/mips/cpu.h       |  2 ++
> >  target/mips/translate.c | 14 ++++++++++++--
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> > index 03c03fd8c6..9ff4a68d90 100644
> > --- a/target/mips/cpu.h
> > +++ b/target/mips/cpu.h
> > @@ -180,6 +180,8 @@ struct TCState {
> >  #define MXU_CR_RD_EN    1
> >  #define MXU_CR_MXU_EN   0
> >
> > +    /* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
> > +    uint64_t mmr[32];
> >  };
> >
> >  typedef struct CPUMIPSState CPUMIPSState;
> > diff --git a/target/mips/translate.c b/target/mips/translate.c
> > index a538351032..9d5150ec8b 100644
> > --- a/target/mips/translate.c
> > +++ b/target/mips/translate.c
> > @@ -2455,7 +2455,10 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
> >  static TCGv_i64 fpu_f64[32];
> >  static TCGv_i64 msa_wr_d[64];
> >
> > -#if !defined(TARGET_MIPS64)
> > +#if defined(TARGET_MIPS64)
> > +/* Upper 64-bit MMRs (multimedia registers); the lower 64-bit are GPRs */
> > +static TCGv_i64 cpu_mmr[32];
> > +#else
> 
> Naming is perfect, but:
> 
> #if-#else-#endif is confusing here:
> - MMR declaration should be under it own #if defined(TARGET_MIPS64)
> - MXU declaration should be under separate #if !defined(TARGET_MIPS64)
> (we are not dealing with two variants (64/32) of the same item, but with two
> separate items, one is valid for 64-bit only, another for 32-bit only)
> 
> >  /* MXU registers */
> >  static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
> >  static TCGv mxu_CR;
> > @@ -29785,7 +29788,14 @@ void mips_tcg_init(void)
> >      fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
> >                                         offsetof(CPUMIPSState, active_fpu.fcr31),
> >                                         "fcr31");
> > -#if !defined(TARGET_MIPS64)
> > +#if defined(TARGET_MIPS64)
> > +    cpu_mmr[0] = NULL;
> > +    for (i = 1; i < 32; i++)
> > +        cpu_mmr[i] = tcg_global_mem_new_i64(cpu_env,
> > +                                            offsetof(CPUMIPSState,
> > +                                                     active_tc.mmr[i]),
> > +                                            regnames[i]);
> > +#else
> >      for (i = 0; i < NUMBER_OF_MXU_REGISTERS - 1; i++) {
> >          mxu_gpr[i] = tcg_global_mem_new(cpu_env,
> >                                          offsetof(CPUMIPSState,
> > --
> 
> The same applies here.
> 
> > 2.19.2
> 
> 

Re: [Qemu-devel] [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions

["Jürgen Urban"]

Hello Aleksandar and Fredrik,

> Gesendet: Mittwoch, 16. Januar 2019 um 20:20 Uhr
> Von: "Aleksandar Markovic" <amarkovic@wavecomp.com>
> An: "Fredrik Noring" <noring@nocrew.org>
> Cc: "Aurelien Jarno" <aurelien@aurel32.net>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, "Jürgen Urban" <JuergenUrban@gmx.de>, "Maciej W. Rozycki" <macro@linux-mips.org>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
> Betreff: Re: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
>
> > From: Fredrik Noring <noring@nocrew.org>
> > Sent: Wednesday, January 16, 2019 4:36 PM
> > To: Aleksandar Markovic
> > Cc: Aurelien Jarno; Philippe Mathieu-Daudé; Jürgen Urban; Maciej W. Rozycki; > qemu-devel@nongnu.org
> > Subject: Re: [PATCH 1/9] target/mips: Require TARGET_MIPS64 for R5900 multimedia instructions
> > 
> > Hi Aleksandar,
> > 
> > > Sorry, I have to disagree with this.
> > 
> > It was, without a doubt, entirely clear that the o32 ABI was going to stay
> > in after this MMI patch series. In other words, this series does not imply
> > the removal of o32. This is obvious, as discussed previously, since the o32
> > ABI is currently the main use case for R5900 QEMU and the reason the R5900
> > was implemented for QEMU to begin with.
> > 
> 
> Fredrik,
> 
> Modeling a 64-bit processor as a 32-bit one should not be a part of QEMU
> upstream.

I tried to find the patch where you have a problem with, but I wasn't able. I don't see a problem in [PATCH 1/9]. So I don't know where the actual problem is coming from.

Let me explain how what the actual system was able to do:
PS2 Linux was using 64-Bit and 128-Bit instructions with ABI o32 since the first day it was released. The toolchains which I released later was also using the feature that the CPU has 128/64-Bit registers which can be used when ABI o32 was used. It is even possible to call Linux ABI n32 syscalls from ABI o32 ELF and vice versa. I used this feature to get around some problems.
The Linux kernel which I released was able to execute MIPS III code (complete instruction set) on R5900 without any problems as long as the addresses were within 32-Bit range, e.g. in ABI n32. All missing instructions including IEEE794 compliance were handled by the Linux exception handlers. It was even able to differ between LL/SC and SQ/LQ, although the same instruction encoding is used.
It was possible to execute unmodified MIPS III ELF files without any problem.
In order to correctly emulate the real PS2 Linux which I released, QEMU has to support 64-Bit registers when using ABI o32 and it would need a way to configure whether all MIPS III instructions should be supported or not.
I.e. QEMU should be able to support ABI o32 with a 64-Bit processor. When this is not working there is a problem with the emulation and the problem is not r5900 related, as ABI o32 code is written in a way that it doesn't matter whether the CPU is 128-Bit (like R5900), 64-Bit or 32-Bit.

Best regards
Jürgen Urban

> > > Processor model must stay the same, and
> > > Linux ABI should not affect, for example, the number and size of processor
> > > registers - just like it is the case in reality.
> > 
> > I thought Maciej's reply to you was quite clear on this topic?
> > 
> > > QEMU is an independent software tool - it is for example, a compiler-agnostic
> > > tool, and the only connection between a compiler and QEMU is the processor
> > > documentation - and this is the reason they work together so well. They shouldn't
> > > be "tweaked" and "integrated" to work together - both QEMU and compiler should
> > > rely only on the processor specification, and should not know anything about the
> > > other side.
> > 
> > The approach was to implement ABIs and instructions that are actually used,
> > and leave unused or optional instructions for later. The reverse, removing
> > ABIs in-use (o32) and focusing on unused instructions (MMIs) does not make
> > sense, so I will obviously not do that.
> > 
> > > My advice for you is to focus on n32 at the time being.
> > >
> > > o32 can be implemented with the same 64-bit processor model, but in a much
> > > different way that you attempted some time ago. To avoid waste of our energy
> > > and time, I am suggesting that we finish n32, and think of o32 in future.
> > 
> > The o32 ABI is more important than n32 now, because o32 is in-use and
> > ready with Glibc, GCC, GAS and the Linux kernel. n32 is easy to enable
> > with a three-line patch, so we can actually use both now.
> > 
> > I recommend that we implement limited support for MMIs for n32 and
> > eventually system mode, and leave it as unsupported for o32 for the time
> > being. [ Perhaps MMIs for o32 could be implemented with 96-bit registers,
> > in contrast to the 64-bit registers for n32, but having LW and LQ without
> > LD seems strange to me. ]
> > 
> > Fredrik
>