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

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

[Craig Janeczek]

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

Craig Janeczek (8):
  target/mips: Introduce MXU registers
  target/mips: Add all MXU opcodes
  target/mips: Add MXU instructions S32I2M and S32M2I
  target/mips: Add MXU instruction S8LDD
  target/mips: Add MXU instruction D16MUL
  target/mips: Add MXU instruction D16MAC
  target/mips: Add MXU instructions Q8MUL and Q8MULSU
  target/mips: Add MXU instructions S32LDD and S32LDDR

 target/mips/cpu.h       |   1 +
 target/mips/translate.c | 601 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 540 insertions(+), 62 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH v3 1/8] 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

 target/mips/cpu.h       |  1 +
 target/mips/translate.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 009202cf64..4b2948a2c8 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -170,6 +170,7 @@ struct TCState {
         MSACSR_FS_MASK)
 
     float_status msa_fp_status;
+    target_ulong mxu_gpr[16];
 };
 
 typedef struct CPUMIPSState CPUMIPSState;
diff --git a/target/mips/translate.c b/target/mips/translate.c
index bdd880bb77..416488b383 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1398,6 +1398,9 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
 
+/* MXU registers */
+static TCGv mxu_gpr[16];
+
 #include "exec/gen-icount.h"
 
 #define gen_helper_0e0i(name, arg) do {                           \
@@ -1517,6 +1520,13 @@ static const char * const msaregnames[] = {
     "w30.d0", "w30.d1", "w31.d0", "w31.d1",
 };
 
+static const char * const mxuregnames[] = {
+    "XR1",  "XR2",  "XR3",  "XR4",  "XR5",
+    "XR6",  "XR7",  "XR8",  "XR9",  "XR10",
+    "XR11", "XR12", "XR13", "XR14", "XR15",
+    "XR16",
+};
+
 #define LOG_DISAS(...)                                                        \
     do {                                                                      \
         if (MIPS_DEBUG_DISAS) {                                               \
@@ -20742,6 +20752,12 @@ void mips_tcg_init(void)
     fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
+
+    for (i = 0; i < 16; i++)
+        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
+                                        offsetof(CPUMIPSState,
+                                                 active_tc.mxu_gpr[i]),
+                                        mxuregnames[i]);
 }
 
 #include "translate_init.inc.c"
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 1/8] target/mips: Introduce MXU registers

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 1/8] target/mips: Introduce MXU registers
> 
> 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
> 
>  target/mips/cpu.h       |  1 +
>  target/mips/translate.c | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 009202cf64..4b2948a2c8 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -170,6 +170,7 @@ struct TCState {
>          MSACSR_FS_MASK)
> 
>      float_status msa_fp_status;
> +    target_ulong mxu_gpr[16];
>  };
> 

Where is the definition of MXU control register (MXU_CR)?

This may be placed in a separate patch, but must be includes in the series.

>  typedef struct CPUMIPSState CPUMIPSState;
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index bdd880bb77..416488b383 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1398,6 +1398,9 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;
>  static TCGv_i64 fpu_f64[32];
>  static TCGv_i64 msa_wr_d[64];
> 
> +/* MXU registers */
> +static TCGv mxu_gpr[16];
> +
>  #include "exec/gen-icount.h"
> 
>  #define gen_helper_0e0i(name, arg) do {                           \
> @@ -1517,6 +1520,13 @@ static const char * const msaregnames[] = {
>      "w30.d0", "w30.d1", "w31.d0", "w31.d1",
>  };
> 
> +static const char * const mxuregnames[] = {
> +    "XR1",  "XR2",  "XR3",  "XR4",  "XR5",
> +    "XR6",  "XR7",  "XR8",  "XR9",  "XR10",
> +    "XR11", "XR12", "XR13", "XR14", "XR15",
> +    "XR16",
> +};

Better alignment:

    "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 +20752,12 @@ void mips_tcg_init(void)
>      fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
>                                         offsetof(CPUMIPSState, active_fpu.fcr31),
>                                         "fcr31");
> +
> +    for (i = 0; i < 16; i++)
> +        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
> +                                        offsetof(CPUMIPSState,
> +                                                 active_tc.mxu_gpr[i]),
> +                                        mxuregnames[i]);

The body of the for loop must be enclosed in { and } (QEMU code style guidelines).

Before sending patches, it is obligatory to run scripts/checkpatch.pl <path-to-your-patches> - this (missing braces) will be reported by this script.

>  }
> 
>  #include "translate_init.inc.c"
> 

Re: [Qemu-devel] [PATCH v3 1/8] target/mips: Introduce MXU registers

[Janeczek, Craig]

Where is the definition of MXU control register (MXU_CR)?
MXU_CR is the last element in this array.


Before sending patches, it is obligatory to run scripts/checkpatch.pl <path-to-your-patches> - this (missing braces) will be reported by this script.
I did run checkpath before sending a patch. This was not reported.

./scripts/checkpatch.pl patch/v3/v3-0001-target-mips-Introduce-MXU-registers.patch 
total: 0 errors, 0 warnings, 41 lines checked

patch/v3/v3-0001-target-mips-Introduce-MXU-registers.patch has no obvious style problems and is ready for submission.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Tuesday, August 28, 2018 10:49 AM
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 v3 1/8] target/mips: Introduce MXU registers


> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 1/8] target/mips: Introduce MXU registers
> 
> 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
> 
>  target/mips/cpu.h       |  1 +
>  target/mips/translate.c | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h index 
> 009202cf64..4b2948a2c8 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -170,6 +170,7 @@ struct TCState {
>          MSACSR_FS_MASK)
> 
>      float_status msa_fp_status;
> +    target_ulong mxu_gpr[16];
>  };
> 

Where is the definition of MXU control register (MXU_CR)?

This may be placed in a separate patch, but must be includes in the series.

>  typedef struct CPUMIPSState CPUMIPSState; diff --git 
> a/target/mips/translate.c b/target/mips/translate.c index 
> bdd880bb77..416488b383 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1398,6 +1398,9 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31;  static 
> TCGv_i64 fpu_f64[32];  static TCGv_i64 msa_wr_d[64];
> 
> +/* MXU registers */
> +static TCGv mxu_gpr[16];
> +
>  #include "exec/gen-icount.h"
> 
>  #define gen_helper_0e0i(name, arg) do {                           \
> @@ -1517,6 +1520,13 @@ static const char * const msaregnames[] = {
>      "w30.d0", "w30.d1", "w31.d0", "w31.d1",  };
> 
> +static const char * const mxuregnames[] = {
> +    "XR1",  "XR2",  "XR3",  "XR4",  "XR5",
> +    "XR6",  "XR7",  "XR8",  "XR9",  "XR10",
> +    "XR11", "XR12", "XR13", "XR14", "XR15",
> +    "XR16",
> +};

Better alignment:

    "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 +20752,12 @@ void mips_tcg_init(void)
>      fpu_fcr31 = tcg_global_mem_new_i32(cpu_env,
>                                         offsetof(CPUMIPSState, active_fpu.fcr31),
>                                         "fcr31");
> +
> +    for (i = 0; i < 16; i++)
> +        mxu_gpr[i] = tcg_global_mem_new(cpu_env,
> +                                        offsetof(CPUMIPSState,
> +                                                 active_tc.mxu_gpr[i]),
> +                                        mxuregnames[i]);

The body of the for loop must be enclosed in { and } (QEMU code style guidelines).

Before sending patches, it is obligatory to run scripts/checkpatch.pl <path-to-your-patches> - this (missing braces) will be reported by this script.

>  }
> 
>  #include "translate_init.inc.c"
> 

[Qemu-devel] [PATCH v3 2/8] target/mips: Add all MXU opcodes

[Craig Janeczek]

Adds all MXU opcodes to the opcode enum. The MXU opcodes overlap with
existing misc and Loongson 2F copcodes. The enums were updated to
reflect the multiple possible meanings where applicable.

Signed-off-by: Craig Janeczek <jancraig@amazon.com>
---
 v1
    - NA
 v2
    - NA
 v3
    - Initial patch, split out from prior first patch

 target/mips/translate.c | 190 +++++++++++++++++++++++++++-------------
 1 file changed, 128 insertions(+), 62 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 416488b383..ae6b16ecd7 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -340,30 +340,80 @@ enum {
 #define MASK_SPECIAL2(op)  MASK_OP_MAJOR(op) | (op & 0x3F)
 
 enum {
+    /* Multiply & xxx operations / MXU */
+    OPC_MADD_MXU_S32MADD   = 0x00 | OPC_SPECIAL2,
+    OPC_MADDU_MXU_S32MADDU = 0x01 | OPC_SPECIAL2,
     /* Multiply & xxx operations */
-    OPC_MADD     = 0x00 | OPC_SPECIAL2,
-    OPC_MADDU    = 0x01 | OPC_SPECIAL2,
-    OPC_MUL      = 0x02 | OPC_SPECIAL2,
-    OPC_MSUB     = 0x04 | OPC_SPECIAL2,
-    OPC_MSUBU    = 0x05 | OPC_SPECIAL2,
+    OPC_MUL                = 0x02 | OPC_SPECIAL2,
+    /* MXU */
+    OPC_MXU_D16MAX         = 0x03 | OPC_SPECIAL2,
+    /* Multiply & xxx operations / MXU */
+    OPC_MSUB_MXU_S32MSUB   = 0x04 | OPC_SPECIAL2,
+    OPC_MSUBU_MXU_S32MSUBU = 0x05 | OPC_SPECIAL2,
+    /* MXU */
+    OPC_MXU_D16AVG  = 0x6 | OPC_SPECIAL2,
+    OPC_MXU_D16CPS  = 0x7 | OPC_SPECIAL2,
+    OPC_MXU_D16MUL  = 0x8 | OPC_SPECIAL2,
+    OPC_MXU_D16MULF = 0x9 | OPC_SPECIAL2,
+    OPC_MXU_D16MAC  = 0xA | OPC_SPECIAL2,
+    OPC_MXU_D16MACF = 0xB | OPC_SPECIAL2,
+    OPC_MXU_D16MADL = 0xC | OPC_SPECIAL2,
+    OPC_MXU_S16MAD  = 0xD | OPC_SPECIAL2,
+    OPC_MXU_Q16ADD  = 0xE | OPC_SPECIAL2,
+    OPC_MXU_D16MACE = 0xF | OPC_SPECIAL2,
+    /* Loongson 2F / MXU */
+    OPC_MULT_G_2F_MXU_S32LDD   = 0x10 | OPC_SPECIAL2,
+    OPC_DMULT_G_2F_MXU_S32STD  = 0x11 | OPC_SPECIAL2,
+    OPC_MULTU_G_2F_MXU_S32LDDV  = 0x12 | OPC_SPECIAL2,
+    OPC_DMULTU_G_2F_MXU_S32STDV = 0x13 | OPC_SPECIAL2,
+    OPC_DIV_G_2F_MXU_S32LDI    = 0x14 | OPC_SPECIAL2,
+    OPC_DDIV_G_2F_MXU_S32SDI   = 0x15 | OPC_SPECIAL2,
+    OPC_DIVU_G_2F_MXU_S32LDIV   = 0x16 | OPC_SPECIAL2,
+    OPC_DDIVU_G_2F_MXU_S32SDIV  = 0x17 | OPC_SPECIAL2,
+    /* MXU */
+    OPC_MXU_D32ADD  = 0x18 | OPC_SPECIAL2,
+    OPC_MXU_D32ACC  = 0x19 | OPC_SPECIAL2,
+    OPC_MXU_Q16ACC  = 0x1B | OPC_SPECIAL2,
+    /* Loongson 2F / MXU */
+    OPC_MOD_G_2F_MXU_Q8ADDE    = 0x1c | OPC_SPECIAL2,
+    OPC_DMOD_G_2F_MXU_Q8ACCE   = 0x1d | OPC_SPECIAL2,
     /* Loongson 2F */
-    OPC_MULT_G_2F   = 0x10 | OPC_SPECIAL2,
-    OPC_DMULT_G_2F  = 0x11 | OPC_SPECIAL2,
-    OPC_MULTU_G_2F  = 0x12 | OPC_SPECIAL2,
-    OPC_DMULTU_G_2F = 0x13 | OPC_SPECIAL2,
-    OPC_DIV_G_2F    = 0x14 | OPC_SPECIAL2,
-    OPC_DDIV_G_2F   = 0x15 | OPC_SPECIAL2,
-    OPC_DIVU_G_2F   = 0x16 | OPC_SPECIAL2,
-    OPC_DDIVU_G_2F  = 0x17 | OPC_SPECIAL2,
-    OPC_MOD_G_2F    = 0x1c | OPC_SPECIAL2,
-    OPC_DMOD_G_2F   = 0x1d | OPC_SPECIAL2,
     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,
-    OPC_DCLZ     = 0x24 | OPC_SPECIAL2,
-    OPC_DCLO     = 0x25 | OPC_SPECIAL2,
+    /* MXU */
+    OPC_MXU_S8LDD = 0x22 | OPC_SPECIAL2,
+    OPC_MXU_S8STD = 0x23 | OPC_SPECIAL2,
+    /* Misc / MXU */
+    OPC_DCLZ_MXU_S8LDI     = 0x24 | OPC_SPECIAL2,
+    OPC_DCLO_MXU_S8SDI     = 0x25 | OPC_SPECIAL2,
+    /* MXU */
+    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,
     /* Special */
     OPC_SDBBP    = 0x3F | OPC_SPECIAL2,
 };
@@ -3563,7 +3613,7 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
         tcg_gen_mulu2_i64(cpu_LO[acc], cpu_HI[acc], t0, t1);
         break;
 #endif
-    case OPC_MADD:
+    case OPC_MADD_MXU_S32MADD:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
             TCGv_i64 t3 = tcg_temp_new_i64();
@@ -3579,7 +3629,7 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
             tcg_temp_free_i64(t2);
         }
         break;
-    case OPC_MADDU:
+    case OPC_MADDU_MXU_S32MADDU:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
             TCGv_i64 t3 = tcg_temp_new_i64();
@@ -3597,7 +3647,7 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
             tcg_temp_free_i64(t2);
         }
         break;
-    case OPC_MSUB:
+    case OPC_MSUB_MXU_S32MSUB:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
             TCGv_i64 t3 = tcg_temp_new_i64();
@@ -3613,7 +3663,7 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
             tcg_temp_free_i64(t2);
         }
         break;
-    case OPC_MSUBU:
+    case OPC_MSUBU_MXU_S32MSUBU:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
             TCGv_i64 t3 = tcg_temp_new_i64();
@@ -3721,7 +3771,7 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
     case OPC_CLO:
     case R6_OPC_CLO:
 #if defined(TARGET_MIPS64)
-    case OPC_DCLO:
+    case OPC_DCLO_MXU_S8SDI:
     case R6_OPC_DCLO:
 #endif
         tcg_gen_not_tl(t0, t0);
@@ -3738,9 +3788,9 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
         tcg_gen_subi_tl(t0, t0, TARGET_LONG_BITS - 32);
         break;
 #if defined(TARGET_MIPS64)
-    case OPC_DCLO:
+    case OPC_DCLO_MXU_S8SDI:
     case R6_OPC_DCLO:
-    case OPC_DCLZ:
+    case OPC_DCLZ_MXU_S8LDI:
     case R6_OPC_DCLZ:
         tcg_gen_clzi_i64(t0, t0, 64);
         break;
@@ -3761,14 +3811,14 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
 
     switch (opc) {
     case OPC_MULT_G_2E:
-    case OPC_MULT_G_2F:
+    case OPC_MULT_G_2F_MXU_S32LDD:
     case OPC_MULTU_G_2E:
-    case OPC_MULTU_G_2F:
+    case OPC_MULTU_G_2F_MXU_S32LDDV:
 #if defined(TARGET_MIPS64)
     case OPC_DMULT_G_2E:
-    case OPC_DMULT_G_2F:
+    case OPC_DMULT_G_2F_MXU_S32STD:
     case OPC_DMULTU_G_2E:
-    case OPC_DMULTU_G_2F:
+    case OPC_DMULTU_G_2F_MXU_S32STDV:
 #endif
         t0 = tcg_temp_new();
         t1 = tcg_temp_new();
@@ -3784,19 +3834,19 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
 
     switch (opc) {
     case OPC_MULT_G_2E:
-    case OPC_MULT_G_2F:
+    case OPC_MULT_G_2F_MXU_S32LDD:
         tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
         tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
         break;
     case OPC_MULTU_G_2E:
-    case OPC_MULTU_G_2F:
+    case OPC_MULTU_G_2F_MXU_S32LDDV:
         tcg_gen_ext32u_tl(t0, t0);
         tcg_gen_ext32u_tl(t1, t1);
         tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
         tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
         break;
     case OPC_DIV_G_2E:
-    case OPC_DIV_G_2F:
+    case OPC_DIV_G_2F_MXU_S32LDI:
         {
             TCGLabel *l1 = gen_new_label();
             TCGLabel *l2 = gen_new_label();
@@ -3818,7 +3868,7 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
         }
         break;
     case OPC_DIVU_G_2E:
-    case OPC_DIVU_G_2F:
+    case OPC_DIVU_G_2F_MXU_S32LDIV:
         {
             TCGLabel *l1 = gen_new_label();
             TCGLabel *l2 = gen_new_label();
@@ -3834,7 +3884,7 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
         }
         break;
     case OPC_MOD_G_2E:
-    case OPC_MOD_G_2F:
+    case OPC_MOD_G_2F_MXU_Q8ADDE:
         {
             TCGLabel *l1 = gen_new_label();
             TCGLabel *l2 = gen_new_label();
@@ -3871,15 +3921,15 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
         break;
 #if defined(TARGET_MIPS64)
     case OPC_DMULT_G_2E:
-    case OPC_DMULT_G_2F:
+    case OPC_DMULT_G_2F_MXU_S32STD:
         tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
         break;
     case OPC_DMULTU_G_2E:
-    case OPC_DMULTU_G_2F:
+    case OPC_DMULTU_G_2F_MXU_S32STDV:
         tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
         break;
     case OPC_DDIV_G_2E:
-    case OPC_DDIV_G_2F:
+    case OPC_DDIV_G_2F_MXU_S32SDI:
         {
             TCGLabel *l1 = gen_new_label();
             TCGLabel *l2 = gen_new_label();
@@ -3898,7 +3948,7 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
         }
         break;
     case OPC_DDIVU_G_2E:
-    case OPC_DDIVU_G_2F:
+    case OPC_DDIVU_G_2F_MXU_S32SDIV:
         {
             TCGLabel *l1 = gen_new_label();
             TCGLabel *l2 = gen_new_label();
@@ -3911,7 +3961,7 @@ static void gen_loongson_integer(DisasContext *ctx, uint32_t opc,
         }
         break;
     case OPC_DMOD_G_2E:
-    case OPC_DMOD_G_2F:
+    case OPC_DMOD_G_2F_MXU_Q8ACCE:
         {
             TCGLabel *l1 = gen_new_label();
             TCGLabel *l2 = gen_new_label();
@@ -13541,16 +13591,32 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
     case 0x2a:
         switch (minor & 3) {
         case MADD_ACC:
-            gen_muldiv(ctx, OPC_MADD, (ctx->opcode >> 14) & 3, rs, rt);
+            gen_muldiv(ctx,
+                       OPC_MADD_MXU_S32MADD,
+                       (ctx->opcode >> 14) & 3,
+                       rs,
+                       rt);
             break;
         case MADDU_ACC:
-            gen_muldiv(ctx, OPC_MADDU, (ctx->opcode >> 14) & 3, rs, rt);
+            gen_muldiv(ctx,
+                       OPC_MADDU_MXU_S32MADDU,
+                       (ctx->opcode >> 14) & 3,
+                       rs,
+                       rt);
             break;
         case MSUB_ACC:
-            gen_muldiv(ctx, OPC_MSUB, (ctx->opcode >> 14) & 3, rs, rt);
+            gen_muldiv(ctx,
+                       OPC_MSUB_MXU_S32MSUB,
+                       (ctx->opcode >> 14) & 3,
+                       rs,
+                       rt);
             break;
         case MSUBU_ACC:
-            gen_muldiv(ctx, OPC_MSUBU, (ctx->opcode >> 14) & 3, rs, rt);
+            gen_muldiv(ctx,
+                       OPC_MSUBU_MXU_S32MSUBU,
+                       (ctx->opcode >> 14) & 3,
+                       rs,
+                       rt);
             break;
         default:
             goto pool32axf_invalid;
@@ -13618,19 +13684,19 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
             break;
         case MADD:
             check_insn_opc_removed(ctx, ISA_MIPS32R6);
-            mips32_op = OPC_MADD;
+            mips32_op = OPC_MADD_MXU_S32MADD;
             goto do_mul;
         case MADDU:
             check_insn_opc_removed(ctx, ISA_MIPS32R6);
-            mips32_op = OPC_MADDU;
+            mips32_op = OPC_MADDU_MXU_S32MADDU;
             goto do_mul;
         case MSUB:
             check_insn_opc_removed(ctx, ISA_MIPS32R6);
-            mips32_op = OPC_MSUB;
+            mips32_op = OPC_MSUB_MXU_S32MSUB;
             goto do_mul;
         case MSUBU:
             check_insn_opc_removed(ctx, ISA_MIPS32R6);
-            mips32_op = OPC_MSUBU;
+            mips32_op = OPC_MSUBU_MXU_S32MSUBU;
         do_mul:
             check_insn(ctx, ISA_MIPS32);
             gen_muldiv(ctx, mips32_op, 0, rs, rt);
@@ -17809,21 +17875,21 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
 
     op1 = MASK_SPECIAL2(ctx->opcode);
     switch (op1) {
-    case OPC_MADD: /* Multiply and add/sub */
-    case OPC_MADDU:
-    case OPC_MSUB:
-    case OPC_MSUBU:
+    case OPC_MADD_MXU_S32MADD: /* Multiply and add/sub */
+    case OPC_MADDU_MXU_S32MADDU:
+    case OPC_MSUB_MXU_S32MSUB:
+    case OPC_MSUBU_MXU_S32MSUBU:
         check_insn(ctx, ISA_MIPS32);
         gen_muldiv(ctx, op1, rd & 3, rs, rt);
         break;
     case OPC_MUL:
         gen_arith(ctx, op1, rd, rs, rt);
         break;
-    case OPC_DIV_G_2F:
-    case OPC_DIVU_G_2F:
-    case OPC_MULT_G_2F:
-    case OPC_MULTU_G_2F:
-    case OPC_MOD_G_2F:
+    case OPC_DIV_G_2F_MXU_S32LDI:
+    case OPC_DIVU_G_2F_MXU_S32LDIV:
+    case OPC_MULT_G_2F_MXU_S32LDD:
+    case OPC_MULTU_G_2F_MXU_S32LDDV:
+    case OPC_MOD_G_2F_MXU_Q8ADDE:
     case OPC_MODU_G_2F:
         check_insn(ctx, INSN_LOONGSON2F);
         gen_loongson_integer(ctx, op1, rd, rs, rt);
@@ -17845,17 +17911,17 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
 #if defined(TARGET_MIPS64)
-    case OPC_DCLO:
-    case OPC_DCLZ:
+    case OPC_DCLO_MXU_S8SDI:
+    case OPC_DCLZ_MXU_S8LDI:
         check_insn(ctx, ISA_MIPS64);
         check_mips_64(ctx);
         gen_cl(ctx, op1, rd, rs);
         break;
-    case OPC_DMULT_G_2F:
-    case OPC_DMULTU_G_2F:
-    case OPC_DDIV_G_2F:
-    case OPC_DDIVU_G_2F:
-    case OPC_DMOD_G_2F:
+    case OPC_DMULT_G_2F_MXU_S32STD:
+    case OPC_DMULTU_G_2F_MXU_S32STDV:
+    case OPC_DDIV_G_2F_MXU_S32SDI:
+    case OPC_DDIVU_G_2F_MXU_S32SDIV:
+    case OPC_DMOD_G_2F_MXU_Q8ACCE:
     case OPC_DMODU_G_2F:
         check_insn(ctx, INSN_LOONGSON2F);
         gen_loongson_integer(ctx, op1, rd, rs, rt);
-- 
2.18.0

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

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 2/8] target/mips: Add all MXU opcodes
> 
> Adds all MXU opcodes to the opcode enum. The MXU opcodes overlap with
> existing misc and Loongson 2F copcodes. The enums were updated to
> reflect the multiple possible meanings where applicable.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>

It is good that all codes are covered, but this situation with overlap is unnatural.

Try creating a separate enum for MXU opcodes, and rearranging switch statement, taking into account what instructions set is supported by the CPU in question.

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

[Janeczek, Craig]

I see that I can check the loongson instructions by checking for INSN_LOONGSON2F. Using MXU if that is not set

What should I check for the mult or misc instructions that were there first?

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Tuesday, August 28, 2018 11:07 AM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v3 2/8] target/mips: Add all MXU opcodes

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 2/8] target/mips: Add all MXU opcodes
> 
> Adds all MXU opcodes to the opcode enum. The MXU opcodes overlap with 
> existing misc and Loongson 2F copcodes. The enums were updated to 
> reflect the multiple possible meanings where applicable.
> 
> Signed-off-by: Craig Janeczek <jancraig@amazon.com>

It is good that all codes are covered, but this situation with overlap is unnatural.

Try creating a separate enum for MXU opcodes, and rearranging switch statement, taking into account what instructions set is supported by the CPU in question.

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

[Aleksandar Markovic]

> I see that I can check the loongson instructions by checking for INSN_LOONGSON2F. Using MXU if that is not set

One more thing to check is MXUEN bit of MXU control register. This should be done before handling any MXU instructions, except S32M2I/S32I2M.

> What should I check for the mult or misc instructions that were there first?

It should be is somewhere in LOONGSON docs.

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

[Janeczek, Craig]

I will re-work each of the mxu_gen_<ins> functions to check for MXUEN and jump over the implementation of the instruction if not enabled.

I would like to clarify the structure of the switch statement before implementing it.

I was originally planning on checking if there was a MXU hit and MXUEN was set, in that case use the MXU instruction, else use the original switch. This will not work as MXUEN is in a TCGv and cant be used in an if statement at that level. That is why I plan on putting the MXUEN check in each of the mxu_gen_<ins> functions.

What should the re-worked switch statement look like which runs the mxu_gen_<ins> functions? 

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Tuesday, August 28, 2018 12:51 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v3 2/8] target/mips: Add all MXU opcodes

> I see that I can check the loongson instructions by checking for INSN_LOONGSON2F. Using MXU if that is not set

One more thing to check is MXUEN bit of MXU control register. This should be done before handling any MXU instructions, except S32M2I/S32I2M.

> What should I check for the mult or misc instructions that were there first?

It should be is somewhere in LOONGSON docs.

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

[Aleksandar Markovic]

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 8:54 PM
>
> Subject: RE: [PATCH v3 2/8] target/mips: Add all MXU opcodes
>
> I will re-work each of the mxu_gen_<ins> functions to check for MXUEN and jump over the implementation of the instruction if not enabled.
> 
> I would like to clarify the structure of the switch statement before implementing it.
>
> I was originally planning on checking if there was a MXU hit and MXUEN was set, in that case use the MXU instruction, else use the original switch. This will not work as MXUEN is in a TCGv and cant be used in an if statement at that level. That is why I plan on putting the MXUEN check in each of the mxu_gen_<ins> functions.
> 
> What should the re-worked switch statement look like which runs the mxu_gen_<ins> functions?

I didn't study these code segments in depth, but at this moment it seems to me that the cleanest organization would be to leave current decode_opc_special2_legacy(env, ctx); as is, and, however, to change the part with its invocation:

    case OPC_SPECIAL2:
        decode_opc_special2_legacy(env, ctx);
        break;
to

    case OPC_SPECIAL2:
        if (<MXU is present>) {
                decode_opc_special2_mxu(env, ctx);
        } else if (<LOONGSON_2F>) {
                decode_opc_special2_legacy(env, ctx);
        }
        break;

where decode_opc_special2_mxu(env, ctx); is the function that you will write.

Or something along these lines...

But I may be mistaken regrding details...

Again, overall, this series is very good. We are arguing about some technical details, out of which many are, frankly, unimportant. I just want to tell you that don't misunderstand my and Richard's critiques - we want this code in QEMU, just want it to be nice, clean, and correct, and we definitely appreciate your work very much!

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

[Janeczek, Craig]

To clarify, if MXU is present (a cpu defined to have MXU instructions is selected) we will never run any of the other special2 commands? That would actually simplify the implementation.

Also I understand the want for clean code, I appreciate the comments.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Tuesday, August 28, 2018 3:35 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v3 2/8] target/mips: Add all MXU opcodes

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 8:54 PM
>
> Subject: RE: [PATCH v3 2/8] target/mips: Add all MXU opcodes
>
> I will re-work each of the mxu_gen_<ins> functions to check for MXUEN and jump over the implementation of the instruction if not enabled.
> 
> I would like to clarify the structure of the switch statement before implementing it.
>
> I was originally planning on checking if there was a MXU hit and MXUEN was set, in that case use the MXU instruction, else use the original switch. This will not work as MXUEN is in a TCGv and cant be used in an if statement at that level. That is why I plan on putting the MXUEN check in each of the mxu_gen_<ins> functions.
> 
> What should the re-worked switch statement look like which runs the mxu_gen_<ins> functions?

I didn't study these code segments in depth, but at this moment it seems to me that the cleanest organization would be to leave current decode_opc_special2_legacy(env, ctx); as is, and, however, to change the part with its invocation:

    case OPC_SPECIAL2:
        decode_opc_special2_legacy(env, ctx);
        break;
to

    case OPC_SPECIAL2:
        if (<MXU is present>) {
                decode_opc_special2_mxu(env, ctx);
        } else if (<LOONGSON_2F>) {
                decode_opc_special2_legacy(env, ctx);
        }
        break;

where decode_opc_special2_mxu(env, ctx); is the function that you will write.

Or something along these lines...

But I may be mistaken regrding details...

Again, overall, this series is very good. We are arguing about some technical details, out of which many are, frankly, unimportant. I just want to tell you that don't misunderstand my and Richard's critiques - we want this code in QEMU, just want it to be nice, clean, and correct, and we definitely appreciate your work very much!

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

[Aleksandar Markovic]

> To clarify, if MXU is present (a cpu defined to have MXU instructions is selected) we will never run any of the other special2 commands? That would actually simplify the implementation.

If presence of "special2_legacy" instructions and presence of MXU instructions are mutually exclusive (and I believe they are (why would they occupy the same opcode space unless they exclude each other?), but at this time don't have enough CPU documentation materials to 100% confirm that), I think this is the way to go.

Thanks,
Aleksandar

[Qemu-devel] [PATCH v3 3/8] 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

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index ae6b16ecd7..f6991aa8ef 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1610,6 +1610,23 @@ static inline void gen_store_gpr (TCGv t, int reg)
         tcg_gen_mov_tl(cpu_gpr[reg], t);
 }
 
+/* MXU General purpose registers moves. */
+static inline void gen_load_mxu_gpr(TCGv t, int reg)
+{
+    if (reg == 0) {
+        tcg_gen_movi_tl(t, 0);
+    } else {
+        tcg_gen_mov_tl(t, mxu_gpr[reg - 1]);
+    }
+}
+
+static inline void gen_store_mxu_gpr(TCGv t, int reg)
+{
+    if (reg != 0) {
+        tcg_gen_mov_tl(mxu_gpr[reg - 1], t);
+    }
+}
+
 /* Moves to/from shadow registers. */
 static inline void gen_load_srsgpr (int from, int to)
 {
@@ -3798,6 +3815,42 @@ 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);
+    gen_store_mxu_gpr(t0, xra);
+
+    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);
+
+    gen_load_mxu_gpr(t0, xra);
+    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)
@@ -17894,6 +17947,15 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         check_insn(ctx, INSN_LOONGSON2F);
         gen_loongson_integer(ctx, op1, rd, rs, rt);
         break;
+
+    case OPC_MXU_S32I2M:
+        gen_mxu_s32i2m(ctx, op1);
+        break;
+
+    case OPC_MXU_S32M2I:
+        gen_mxu_s32m2i(ctx, op1);
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I
> 
> 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
> 
>  target/mips/translate.c | 62 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index ae6b16ecd7..f6991aa8ef 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1610,6 +1610,23 @@ static inline void gen_store_gpr (TCGv t, int reg)
>          tcg_gen_mov_tl(cpu_gpr[reg], t);
>  }
> 
> +/* MXU General purpose registers moves. */
> +static inline void gen_load_mxu_gpr(TCGv t, int reg)
> +{
> +    if (reg == 0) {
> +        tcg_gen_movi_tl(t, 0);
> +    } else {
> +        tcg_gen_mov_tl(t, mxu_gpr[reg - 1]);
> +    }
> +}

What happens if reg > 16? Also, the argument reg should be unsigned.

> +
> +static inline void gen_store_mxu_gpr(TCGv t, int reg)
> +{
> +    if (reg != 0) {
> +        tcg_gen_mov_tl(mxu_gpr[reg - 1], t);
> +    }
> +}
> +

What happens if reg > 16? Also, the argument reg should be unsigned.

>  /* Moves to/from shadow registers. */
>  static inline void gen_load_srsgpr (int from, int to)
>  {
> @@ -3798,6 +3815,42 @@ 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);
> +    gen_store_mxu_gpr(t0, xra);
> +
> +    tcg_temp_free(t0);
> +}

This does not handle the case xra == XR16. From the doc:

"In MXU, a dedicated register file named XRF comprises sixteen 32-bit general purpose registers – XR0~XR15. XR0 is a special one, which always is read as zero. Moreover, XR16 is an alias of MXU_CR described below and it can only be accessed by S32I2M/S32M2I instructions."

> +
> +/* 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);
> +
> +    gen_load_mxu_gpr(t0, xra);
> +    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)
> @@ -17894,6 +17947,15 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>          check_insn(ctx, INSN_LOONGSON2F);
>          gen_loongson_integer(ctx, op1, rd, rs, rt);
>          break;
> +
> +    case OPC_MXU_S32I2M:
> +        gen_mxu_s32i2m(ctx, op1);
> +        break;
> +
> +    case OPC_MXU_S32M2I:
> +        gen_mxu_s32m2i(ctx, op1);
> +        break;
> +
>      case OPC_CLO:
>      case OPC_CLZ:
>          check_insn(ctx, ISA_MIPS32);

Re: [Qemu-devel] [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

[Janeczek, Craig]

What happens if reg > 16? Also, the argument reg should be unsigned.
If rev > 16 the instruction is invalid. What type of error can/should I throw here.

This does not handle the case xra == XR16. From the doc:
I do not see where the case is un-handled. XR16 maps to index 15 in the mxu_gpr array. 

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Tuesday, August 28, 2018 10:43 AM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and 
> S32M2I
> 
> 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
> 
>  target/mips/translate.c | 62 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> ae6b16ecd7..f6991aa8ef 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1610,6 +1610,23 @@ static inline void gen_store_gpr (TCGv t, int reg)
>          tcg_gen_mov_tl(cpu_gpr[reg], t);  }
> 
> +/* MXU General purpose registers moves. */ static inline void 
> +gen_load_mxu_gpr(TCGv t, int reg) {
> +    if (reg == 0) {
> +        tcg_gen_movi_tl(t, 0);
> +    } else {
> +        tcg_gen_mov_tl(t, mxu_gpr[reg - 1]);
> +    }
> +}

What happens if reg > 16? Also, the argument reg should be unsigned.

> +
> +static inline void gen_store_mxu_gpr(TCGv t, int reg) {
> +    if (reg != 0) {
> +        tcg_gen_mov_tl(mxu_gpr[reg - 1], t);
> +    }
> +}
> +

What happens if reg > 16? Also, the argument reg should be unsigned.

>  /* Moves to/from shadow registers. */  static inline void 
> gen_load_srsgpr (int from, int to)  { @@ -3798,6 +3815,42 @@ 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);
> +    gen_store_mxu_gpr(t0, xra);
> +
> +    tcg_temp_free(t0);
> +}

This does not handle the case xra == XR16. From the doc:

"In MXU, a dedicated register file named XRF comprises sixteen 32-bit general purpose registers - XR0~XR15. XR0 is a special one, which always is read as zero. Moreover, XR16 is an alias of MXU_CR described below and it can only be accessed by S32I2M/S32M2I instructions."

> +
> +/* 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);
> +
> +    gen_load_mxu_gpr(t0, xra);
> +    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) @@ -17894,6 
> +17947,15 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>          check_insn(ctx, INSN_LOONGSON2F);
>          gen_loongson_integer(ctx, op1, rd, rs, rt);
>          break;
> +
> +    case OPC_MXU_S32I2M:
> +        gen_mxu_s32i2m(ctx, op1);
> +        break;
> +
> +    case OPC_MXU_S32M2I:
> +        gen_mxu_s32m2i(ctx, op1);
> +        break;
> +
>      case OPC_CLO:
>      case OPC_CLZ:
>          check_insn(ctx, ISA_MIPS32);

Re: [Qemu-devel] [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

[Aleksandar Markovic]

> > This does not handle the case xra == XR16.

> I do not see where the case is un-handled. XR16 maps to index 15 in the mxu_gpr array.

But, XR16 has its own rules for read/write, and you are treating it just as a regular register. 

Re: [Qemu-devel] [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

[Janeczek, Craig]

The only commands that have the 5th bit required to address XR16 are S32M2I/S32I2M.

I can split it out into a separate utility function and put a conditional into the S32M2I/S32I2M functions if you are more comfortable with that.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Tuesday, August 28, 2018 12:53 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

> > This does not handle the case xra == XR16.

> I do not see where the case is un-handled. XR16 maps to index 15 in the mxu_gpr array.

But, XR16 has its own rules for read/write, and you are treating it just as a regular register. 

Re: [Qemu-devel] [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

[Aleksandar Markovic]

> The only commands that have the 5th bit required to address XR16 are S32M2I/S32I2M.
> 
> I can split it out into a separate utility function and put a conditional into the S32M2I/S32I2M functions if you are more comfortable with that.

It is not a bad idea. (preventing all instructions other than S32M2I/S32I2M from accessing XR16)

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com>
Sent: Tuesday, August 28, 2018 12:53 PM
To: Janeczek, Craig <jancraig@amazon.com>; qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [PATCH v3 3/8] target/mips: Add MXU instructions S32I2M and S32M2I

> > This does not handle the case xra == XR16.

> I do not see where the case is un-handled. XR16 maps to index 15 in the mxu_gpr array.

But, XR16 has its own rules for read/write, and you are treating it just as a regular register.

[Qemu-devel] [PATCH v3 4/8] 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

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index f6991aa8ef..024e48baf6 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3851,6 +3851,71 @@ 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;
+    uint32_t xra, s8, optn3, rb;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+
+    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_gpr(t0, rb);
+    tcg_gen_addi_tl(t0, t0, (int8_t)s8);
+    switch (optn3) {
+    case 0: /*XRa[7:0] = tmp8 */
+        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;
+    case 1: /* XRa[15:8] = tmp8 */
+        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;
+    case 2: /* XRa[23:16] = tmp8 */
+        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;
+    case 3: /* XRa[31:24] = tmp8 */
+        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;
+    case 4: /* XRa = {8'b0, tmp8, 8'b0, tmp8} */
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
+        tcg_gen_deposit_tl(t0, t1, t1, 16, 16);
+        break;
+    case 5: /* XRa = {tmp8, 8'b0, tmp8, 8'b0} */
+        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;
+    case 6: /* XRa = {{8{sign of tmp8}}, tmp8, {8{sign of tmp8}}, tmp8} */
+        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;
+    case 7: /* XRa = {tmp8, tmp8, tmp8, tmp8} */
+        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);
+
+    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)
@@ -17956,6 +18021,10 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_s32m2i(ctx, op1);
         break;
 
+    case OPC_MXU_S8LDD:
+        gen_mxu_s8ldd(ctx, op1);
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 4/8] target/mips: Add MXU instruction S8LDD

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 4/8] 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
> 
>  target/mips/translate.c | 69 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index f6991aa8ef..024e48baf6 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -3851,6 +3851,71 @@ 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;
> +    uint32_t xra, s8, optn3, rb;
> +
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +
> +    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_gpr(t0, rb);
> +    tcg_gen_addi_tl(t0, t0, (int8_t)s8);
> +    switch (optn3) {
> +    case 0: /*XRa[7:0] = tmp8 */
> +        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;
> +    case 1: /* XRa[15:8] = tmp8 */
> +        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;
> +    case 2: /* XRa[23:16] = tmp8 */
> +        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;
> +    case 3: /* XRa[31:24] = tmp8 */
> +        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;
> +    case 4: /* XRa = {8'b0, tmp8, 8'b0, tmp8} */
> +        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
> +        tcg_gen_deposit_tl(t0, t1, t1, 16, 16);
> +        break;
> +    case 5: /* XRa = {tmp8, 8'b0, tmp8, 8'b0} */
> +        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;
> +    case 6: /* XRa = {{8{sign of tmp8}}, tmp8, {8{sign of tmp8}}, tmp8} */
> +        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;
> +    case 7: /* XRa = {tmp8, tmp8, tmp8, tmp8} */
> +        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;
> +    }

Similar to the situation in patch 6, the following is missing:

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

> +    gen_store_mxu_gpr(t0, xra);
> +
> +    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)
> @@ -17956,6 +18021,10 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>          gen_mxu_s32m2i(ctx, op1);
>          break;
> 
> +    case OPC_MXU_S8LDD:
> +        gen_mxu_s8ldd(ctx, op1);
> +        break;
> +
>      case OPC_CLO:
>      case OPC_CLZ:
>          check_insn(ctx, ISA_MIPS32);
> --
> 2.18.0
> 

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v3 4/8] target/mips: Add MXU instruction S8LDD

[Richard Henderson]

On 08/28/2018 06:00 AM, Craig Janeczek via Qemu-devel wrote:
> +    case 6: /* XRa = {{8{sign of tmp8}}, tmp8, {8{sign of tmp8}}, tmp8} */
> +        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;

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


r~

[Qemu-devel] [PATCH v3 5/8] 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

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 024e48baf6..f693e45203 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3916,6 +3916,57 @@ 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;
+    uint32_t xra, xrb, xrc, xrd, optn2;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+    t2 = tcg_temp_new();
+    t3 = tcg_temp_new();
+
+    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_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 0: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t1, t3);
+        tcg_gen_mul_tl(t2, t0, t2);
+        break;
+    case 1: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t0, t3);
+        tcg_gen_mul_tl(t2, t0, t2);
+        break;
+    case 2: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t1, t3);
+        tcg_gen_mul_tl(t2, t1, t2);
+        break;
+    case 3: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t0, t3);
+        tcg_gen_mul_tl(t2, t1, t2);
+        break;
+    }
+    gen_store_mxu_gpr(t3, xra);
+    gen_store_mxu_gpr(t2, xrd);
+
+    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)
@@ -18025,6 +18076,10 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_s8ldd(ctx, op1);
         break;
 
+    case OPC_MXU_D16MUL:
+        gen_mxu_d16mul(ctx, op1);
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

[Qemu-devel] [PATCH v3 6/8] 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

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index f693e45203..7c17867d30 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3967,6 +3967,80 @@ 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;
+    uint32_t xra, xrb, xrc, xrd, optn2, aptn2;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+    t2 = tcg_temp_new();
+    t3 = tcg_temp_new();
+
+    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_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 0: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t1, t3);
+        tcg_gen_mul_tl(t2, t0, t2);
+        break;
+    case 1: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t0, t3);
+        tcg_gen_mul_tl(t2, t0, t2);
+        break;
+    case 2: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t1, t3);
+        tcg_gen_mul_tl(t2, t1, t2);
+        break;
+    case 3: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */
+        tcg_gen_mul_tl(t3, t0, t3);
+        tcg_gen_mul_tl(t2, t1, t2);
+        break;
+    }
+    gen_load_mxu_gpr(t0, xra);
+    gen_load_mxu_gpr(t1, xrd);
+
+    switch (aptn2) {
+    case 0:
+        tcg_gen_add_tl(t3, t0, t3);
+        tcg_gen_add_tl(t2, t1, t2);
+        break;
+    case 1:
+        tcg_gen_add_tl(t3, t0, t3);
+        tcg_gen_sub_tl(t2, t1, t2);
+        break;
+    case 2:
+        tcg_gen_sub_tl(t3, t0, t3);
+        tcg_gen_add_tl(t2, t1, t2);
+        break;
+    case 3:
+        tcg_gen_sub_tl(t3, t0, t3);
+        tcg_gen_sub_tl(t2, t1, t2);
+        break;
+    }
+    gen_store_mxu_gpr(t3, xra);
+    gen_store_mxu_gpr(t2, xrd);
+
+    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)
@@ -18080,6 +18154,10 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_d16mul(ctx, op1);
         break;
 
+    case OPC_MXU_D16MAC:
+        gen_mxu_d16mac(ctx, op1);
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 6/8] target/mips: Add MXU instruction D16MAC

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic; aurelien@aurel32.net; Craig Janeczek
> Subject: [PATCH v3 6/8] target/mips: Add MXU instruction D16MAC
> 
> 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
> 
>  target/mips/translate.c | 78 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index f693e45203..7c17867d30 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -3967,6 +3967,80 @@ 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 */

In QEMU, for all new code, the multiline comment should be in this format:

/*
 * This is a
 * multiline comment.
 */

Sometimes it is allowed to wtite this:

/*
 * Single line comment in multiline format.
 */

"Classic" comment format is reserved just for such single-line cases:

/* This is a classic way to wtite a comment. */

> +static void gen_mxu_d16mac(DisasContext *ctx, uint32_t opc)
> +{
> +    TCGv t0, t1, t2, t3;
> +    uint32_t xra, xrb, xrc, xrd, optn2, aptn2;
> +
> +    t0 = tcg_temp_new();
> +    t1 = tcg_temp_new();
> +    t2 = tcg_temp_new();
> +    t3 = tcg_temp_new();
> +
> +    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_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 0: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */
> +        tcg_gen_mul_tl(t3, t1, t3);
> +        tcg_gen_mul_tl(t2, t0, t2);
> +        break;
> +    case 1: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */
> +        tcg_gen_mul_tl(t3, t0, t3);
> +        tcg_gen_mul_tl(t2, t0, t2);
> +        break;
> +    case 2: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */
> +        tcg_gen_mul_tl(t3, t1, t3);
> +        tcg_gen_mul_tl(t2, t1, t2);
> +        break;
> +    case 3: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */
> +        tcg_gen_mul_tl(t3, t0, t3);
> +        tcg_gen_mul_tl(t2, t1, t2);
> +        break;
> +    }

Somewhere above all gem_mxu_XXX() functions, the following code segment should be inserted:

/* MXU operand getting patterns */
#define MXU_OPTN2_WW    0
#define MXU_OPTN2_LW    1
#define MXU_OPTN2_HW    2
#define MXU_OPTN2_XW    3

... and these constants should be used in the switch statement just above.

> +    gen_load_mxu_gpr(t0, xra);
> +    gen_load_mxu_gpr(t1, xrd);
> +
> +    switch (aptn2) {
> +    case 0:
> +        tcg_gen_add_tl(t3, t0, t3);
> +        tcg_gen_add_tl(t2, t1, t2);
> +        break;
> +    case 1:
> +        tcg_gen_add_tl(t3, t0, t3);
> +        tcg_gen_sub_tl(t2, t1, t2);
> +        break;
> +    case 2:
> +        tcg_gen_sub_tl(t3, t0, t3);
> +        tcg_gen_add_tl(t2, t1, t2);
> +        break;
> +    case 3:
> +        tcg_gen_sub_tl(t3, t0, t3);
> +        tcg_gen_sub_tl(t2, t1, t2);
> +        break;
> +    }

Somewhere above all gem_mxu_XXX() functions, the following code segment should be inserted:

/* MXU acumulate patterns */
#define MXU_APTN2_AA    0
#define MXU_APTN2_AS    1
#define MXU_APTN2_SA    2
#define MXU_APTN2_SS    3

... and these constants should be used in the switch statement just above.

> +    gen_store_mxu_gpr(t3, xra);
> +    gen_store_mxu_gpr(t2, xrd);
> +
> +    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)
> @@ -18080,6 +18154,10 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
>          gen_mxu_d16mul(ctx, op1);
>          break;
> 
> +    case OPC_MXU_D16MAC:
> +        gen_mxu_d16mac(ctx, op1);
> +        break;
> +
>      case OPC_CLO:
>      case OPC_CLZ:
>          check_insn(ctx, ISA_MIPS32);

[Qemu-devel] [PATCH v3 7/8] 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

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

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 7c17867d30..a28ea124c2 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4041,6 +4041,88 @@ 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;
+    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();
+
+    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_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);
+
+    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)
@@ -18158,6 +18240,10 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_d16mac(ctx, op1);
         break;
 
+    case OPC_MXU_Q8MUL:
+        gen_mxu_q8mul(ctx, op1);
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

[Qemu-devel] [PATCH v3 8/8] 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

 target/mips/translate.c | 47 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index a28ea124c2..1ddc9f04e8 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -4123,6 +4123,42 @@ 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;
+    uint32_t xra, s12, sel, rb;
+
+    t0 = tcg_temp_new();
+    t1 = tcg_temp_new();
+
+    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_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);
+
+    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)
@@ -18212,7 +18248,6 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         break;
     case OPC_DIV_G_2F_MXU_S32LDI:
     case OPC_DIVU_G_2F_MXU_S32LDIV:
-    case OPC_MULT_G_2F_MXU_S32LDD:
     case OPC_MULTU_G_2F_MXU_S32LDDV:
     case OPC_MOD_G_2F_MXU_Q8ADDE:
     case OPC_MODU_G_2F:
@@ -18244,6 +18279,16 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
         gen_mxu_q8mul(ctx, op1);
         break;
 
+    case OPC_MULT_G_2F_MXU_S32LDD:
+        /* There is an overlap of opcodes between Loongson2F and MXU */
+        if (ctx->insn_flags & INSN_LOONGSON2F) {
+            check_insn(ctx, INSN_LOONGSON2F);
+            gen_loongson_integer(ctx, op1, rd, rs, rt);
+        } else {
+            gen_mxu_s32ldd(ctx, op1);
+        }
+        break;
+
     case OPC_CLO:
     case OPC_CLZ:
         check_insn(ctx, ISA_MIPS32);
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 0/8] Add limited MXU instruction support

[Aleksandar Markovic]

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
>
> Subject: [PATCH v3 0/8] Add limited MXU instruction support
>
> This patch set begins to add MXU instruction support for mips emulation.

Craig,

May I ask you what is the exact platform (CPU/board) that you are targeting with this series? Do we need perhaps to add/modify something in translate_init.inc.c? I am very curious.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v3 0/8] Add limited MXU instruction support

[Aleksandar Markovic]

Hi, Craig,

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> 
> Subject: [PATCH v3 0/8] Add limited MXU instruction support
> 
> This patch set begins to add MXU instruction support for mips emulation.

Based on the info I have, I think a reasonable approach to integration of this series would be:

- Add this line in mips-defs.h

#define   ASE_MXU       0x02000000

- In main switch, use this segment

        if (ctx->insn_flags & ASE_MXU) {
                decode_opc_special2_mxu(env, ctx);
        } else {
                decode_opc_special2_legacy(env, ctx);
        }

That way, you would be able to add MXU code without specifying CPU that supports it. This will enable you to focus on MXU, which is a serieoous task anyway. Hopefully, a CPU will be added at some later date.

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH v3 0/8] Add limited MXU instruction support

[Janeczek, Craig]

That seems like a good plan to separate these tasks. I will implement this in the next patchset. Thank you

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Thursday, August 30, 2018 8:40 AM
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 v3 0/8] Add limited MXU instruction support

Hi, Craig,

> From: Craig Janeczek <jancraig@amazon.com>
> Sent: Tuesday, August 28, 2018 3:00 PM
> 
> Subject: [PATCH v3 0/8] Add limited MXU instruction support
> 
> This patch set begins to add MXU instruction support for mips emulation.

Based on the info I have, I think a reasonable approach to integration of this series would be:

- Add this line in mips-defs.h

#define   ASE_MXU       0x02000000

- In main switch, use this segment

        if (ctx->insn_flags & ASE_MXU) {
                decode_opc_special2_mxu(env, ctx);
        } else {
                decode_opc_special2_legacy(env, ctx);
        }

That way, you would be able to add MXU code without specifying CPU that supports it. This will enable you to focus on MXU, which is a serieoous task anyway. Hopefully, a CPU will be added at some later date.

Thanks,
Aleksandar