All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support
@ 2018-12-17 20:04 Aleksandar Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions Aleksandar Markovic
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Various updates to MXU ASE support.

Aleksandar Markovic (6):
  target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  target/mips: MXU: Add generic naming for optn2 constants
  target/mips: MXU: Improve textual description
  target/mips: MXU: Add handlers for logic instructions
  target/mips: MXU: Add handlers for max/min instructions
  target/mips: MXU: Add handlers for an align instruction

 target/mips/translate.c | 954 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 850 insertions(+), 104 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
@ 2018-12-17 20:04 ` Aleksandar Markovic
  2018-12-18 14:21   ` Stefan Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 2/6] target/mips: MXU: Add generic naming for optn2 constants Aleksandar Markovic
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Add missing opcodes and decoding engine for LXB, LXH, LXW, LXBU,
and LXHU instructions. They were for some reason forgotten in
previous commits. The MXU opcode list and decoding engine should
be now complete.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 140 +++++++++++++++++++++++++++++-----------
 1 file changed, 102 insertions(+), 38 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index e9c23a594b..e0c8d8c2f7 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1663,12 +1663,21 @@ enum {
  *          │                               20..18
  *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
  *          │                            ├─ 001 ─ OPC_MXU_S32ALN
- *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
- *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
- *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
- *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
- *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
- *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
+ *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
+ *          │                            ├─ 011 ─ OPC_MXU_S32NOR
+ *          │                            ├─ 100 ─ OPC_MXU_S32AND
+ *          │                            ├─ 101 ─ OPC_MXU_S32OR
+ *          │                            ├─ 110 ─ OPC_MXU_S32XOR
+ *          │                            └─ 111 ─ OPC_MXU_S32LUI
+ *          │
+ *          │                               7..5
+ *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
+ *          │                            ├─ 001 ─ OPC_MXU_LXH
+ *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
+ *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
+ *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
+ *          ├─ 101100 ─ OPC_MXU_S16LDI
+ *          ├─ 101101 ─ OPC_MXU_S16SDI
  *          ├─ 101110 ─ OPC_MXU_S32M2I
  *          ├─ 101111 ─ OPC_MXU_S32I2M
  *          ├─ 110000 ─ OPC_MXU_D32SLL
@@ -1678,15 +1687,15 @@ enum {
  *          ├─ 110100 ─ OPC_MXU_Q16SLL   ├─ 010 ─ OPC_MXU_D32SARV
  *          ├─ 110101 ─ OPC_MXU_Q16SLR   ├─ 011 ─ OPC_MXU_Q16SLLV
  *          │                            ├─ 100 ─ OPC_MXU_Q16SLRV
- *          ├─ 110110 ─ OPC_MXU__POOL17 ─┴─ 101 ─ OPC_MXU_Q16SARV
+ *          ├─ 110110 ─ OPC_MXU__POOL18 ─┴─ 101 ─ OPC_MXU_Q16SARV
  *          │
  *          ├─ 110111 ─ OPC_MXU_Q16SAR
  *          │                               23..22
- *          ├─ 111000 ─ OPC_MXU__POOL18 ─┬─ 00 ─ OPC_MXU_Q8MUL
+ *          ├─ 111000 ─ OPC_MXU__POOL19 ─┬─ 00 ─ OPC_MXU_Q8MUL
  *          │                            └─ 01 ─ OPC_MXU_Q8MULSU
  *          │
  *          │                               20..18
- *          ├─ 111001 ─ OPC_MXU__POOL19 ─┬─ 000 ─ OPC_MXU_Q8MOVZ
+ *          ├─ 111001 ─ OPC_MXU__POOL20 ─┬─ 000 ─ OPC_MXU_Q8MOVZ
  *          │                            ├─ 001 ─ OPC_MXU_Q8MOVN
  *          │                            ├─ 010 ─ OPC_MXU_D16MOVZ
  *          │                            ├─ 011 ─ OPC_MXU_D16MOVN
@@ -1694,7 +1703,7 @@ enum {
  *          │                            └─ 101 ─ OPC_MXU_S32MOV
  *          │
  *          │                               23..22
- *          ├─ 111010 ─ OPC_MXU__POOL20 ─┬─ 00 ─ OPC_MXU_Q8MAC
+ *          ├─ 111010 ─ OPC_MXU__POOL21 ─┬─ 00 ─ OPC_MXU_Q8MAC
  *          │                            └─ 10 ─ OPC_MXU_Q8MACSU
  *          ├─ 111011 ─ OPC_MXU_Q16SCOP
  *          ├─ 111100 ─ OPC_MXU_Q8MADL
@@ -1750,7 +1759,7 @@ enum {
     OPC_MXU_S8SDI    = 0x25,
     OPC_MXU__POOL15  = 0x26,
     OPC_MXU__POOL16  = 0x27,
-    OPC_MXU_LXB      = 0x28,
+    OPC_MXU__POOL17  = 0x28,
     /* not assigned 0x29 */
     OPC_MXU_S16LDD   = 0x2A,
     OPC_MXU_S16STD   = 0x2B,
@@ -1764,11 +1773,11 @@ enum {
     OPC_MXU_D32SAR   = 0x33,
     OPC_MXU_Q16SLL   = 0x34,
     OPC_MXU_Q16SLR   = 0x35,
-    OPC_MXU__POOL17  = 0x36,
+    OPC_MXU__POOL18  = 0x36,
     OPC_MXU_Q16SAR   = 0x37,
-    OPC_MXU__POOL18  = 0x38,
-    OPC_MXU__POOL19  = 0x39,
-    OPC_MXU__POOL20  = 0x3A,
+    OPC_MXU__POOL19  = 0x38,
+    OPC_MXU__POOL20  = 0x39,
+    OPC_MXU__POOL21  = 0x3A,
     OPC_MXU_Q16SCOP  = 0x3B,
     OPC_MXU_Q8MADL   = 0x3C,
     OPC_MXU_S32SFL   = 0x3D,
@@ -1940,6 +1949,17 @@ enum {
 /*
  * MXU pool 17
  */
+enum {
+    OPC_MXU_LXB      = 0x00,
+    OPC_MXU_LXH      = 0x01,
+    OPC_MXU_LXW      = 0x03,
+    OPC_MXU_LXBU     = 0x04,
+    OPC_MXU_LXHU     = 0x05,
+};
+
+/*
+ * MXU pool 18
+ */
 enum {
     OPC_MXU_D32SLLV  = 0x00,
     OPC_MXU_D32SLRV  = 0x01,
@@ -1950,7 +1970,7 @@ enum {
 };
 
 /*
- * MXU pool 18
+ * MXU pool 19
  */
 enum {
     OPC_MXU_Q8MUL    = 0x00,
@@ -1958,7 +1978,7 @@ enum {
 };
 
 /*
- * MXU pool 19
+ * MXU pool 20
  */
 enum {
     OPC_MXU_Q8MOVZ   = 0x00,
@@ -1970,7 +1990,7 @@ enum {
 };
 
 /*
- * MXU pool 20
+ * MXU pool 21
  */
 enum {
     OPC_MXU_Q8MAC    = 0x00,
@@ -25331,12 +25351,58 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
  * Decode MXU pool17
  *
  *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+---------+---+---------+-----+-----------+
+ *  |  SPECIAL2 |    rs   |    rt   |0 0|    rd   |x x x|MXU__POOL15|
+ *  +-----------+---------+---------+---+---------+-----+-----------+
+ *
+ */
+static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
+{
+    uint32_t opcode = extract32(ctx->opcode, 6, 2);
+
+    switch (opcode) {
+    case OPC_MXU_LXW:
+        /* TODO: Implement emulation of LXW instruction. */
+        MIPS_INVAL("OPC_MXU_LXW");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    case OPC_MXU_LXH:
+        /* TODO: Implement emulation of LXH instruction. */
+        MIPS_INVAL("OPC_MXU_LXH");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    case OPC_MXU_LXHU:
+        /* TODO: Implement emulation of LXHU instruction. */
+        MIPS_INVAL("OPC_MXU_LXHU");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    case OPC_MXU_LXB:
+        /* TODO: Implement emulation of LXB instruction. */
+        MIPS_INVAL("OPC_MXU_LXB");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    case OPC_MXU_LXBU:
+        /* TODO: Implement emulation of LXBU instruction. */
+        MIPS_INVAL("OPC_MXU_LXBU");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    default:
+        MIPS_INVAL("decode_opc_mxu");
+        generate_exception_end(ctx, EXCP_RI);
+        break;
+    }
+}
+/*
+ *
+ * Decode MXU pool18
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
  *  +-----------+---------+-----+-------+-------+-------+-----------+
- *  |  SPECIAL2 |    rb   |x x x|  XRd  |  XRa  |0 0 0 0|MXU__POOL17|
+ *  |  SPECIAL2 |    rb   |x x x|  XRd  |  XRa  |0 0 0 0|MXU__POOL18|
  *  +-----------+---------+-----+-------+-------+-------+-----------+
  *
  */
-static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
+static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opcode = extract32(ctx->opcode, 18, 3);
 
@@ -25380,15 +25446,15 @@ static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
 
 /*
  *
- * Decode MXU pool18
+ * Decode MXU pool19
  *
  *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
  *  +-----------+---+---+-------+-------+-------+-------+-----------+
- *  |  SPECIAL2 |0 0|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL18|
+ *  |  SPECIAL2 |0 0|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL19|
  *  +-----------+---+---+-------+-------+-------+-------+-----------+
  *
  */
-static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
+static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opcode = extract32(ctx->opcode, 22, 2);
 
@@ -25406,15 +25472,15 @@ static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
 
 /*
  *
- * Decode MXU pool19
+ * Decode MXU pool20
  *
  *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
  *  +-----------+---------+-----+-------+-------+-------+-----------+
- *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL19|
+ *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL20|
  *  +-----------+---------+-----+-------+-------+-------+-----------+
  *
  */
-static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
+static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opcode = extract32(ctx->opcode, 18, 3);
 
@@ -25458,15 +25524,15 @@ static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
 
 /*
  *
- * Decode MXU pool20
+ * Decode MXU pool21
  *
  *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
  *  +-----------+---+---+-------+-------+-------+-------+-----------+
- *  |  SPECIAL2 |an2|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL20|
+ *  |  SPECIAL2 |an2|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL21|
  *  +-----------+---+---+-------+-------+-------+-------+-----------+
  *
  */
-static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext *ctx)
+static void decode_opc_mxu__pool21(CPUMIPSState *env, DisasContext *ctx)
 {
     uint32_t opcode = extract32(ctx->opcode, 22, 2);
 
@@ -25669,10 +25735,8 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
         case OPC_MXU__POOL16:
             decode_opc_mxu__pool16(env, ctx);
             break;
-        case OPC_MXU_LXB:
-            /* TODO: Implement emulation of LXB instruction. */
-            MIPS_INVAL("OPC_MXU_LXB");
-            generate_exception_end(ctx, EXCP_RI);
+        case OPC_MXU__POOL17:
+            decode_opc_mxu__pool17(env, ctx);
             break;
         case OPC_MXU_S16LDD:
             /* TODO: Implement emulation of S16LDD instruction. */
@@ -25724,23 +25788,23 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
             MIPS_INVAL("OPC_MXU_Q16SLR");
             generate_exception_end(ctx, EXCP_RI);
             break;
-        case OPC_MXU__POOL17:
-            decode_opc_mxu__pool17(env, ctx);
+        case OPC_MXU__POOL18:
+            decode_opc_mxu__pool18(env, ctx);
             break;
         case OPC_MXU_Q16SAR:
             /* TODO: Implement emulation of Q16SAR instruction. */
             MIPS_INVAL("OPC_MXU_Q16SAR");
             generate_exception_end(ctx, EXCP_RI);
             break;
-        case OPC_MXU__POOL18:
-            decode_opc_mxu__pool18(env, ctx);
-            break;
         case OPC_MXU__POOL19:
             decode_opc_mxu__pool19(env, ctx);
             break;
         case OPC_MXU__POOL20:
             decode_opc_mxu__pool20(env, ctx);
             break;
+        case OPC_MXU__POOL21:
+            decode_opc_mxu__pool21(env, ctx);
+            break;
         case OPC_MXU_Q16SCOP:
             /* TODO: Implement emulation of Q16SCOP instruction. */
             MIPS_INVAL("OPC_MXU_Q16SCOP");
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 2/6] target/mips: MXU: Add generic naming for optn2 constants
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions Aleksandar Markovic
@ 2018-12-17 20:04 ` Aleksandar Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 3/6] target/mips: MXU: Improve textual description Aleksandar Markovic
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Add generic naming involving generig suffixes OPTN0, OPTN1,
OPTN2, OPTN3 for four optn2 constants. Suffixes WW, LW, HW,
XW are not quite appropriate for some instructions using optn2.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index e0c8d8c2f7..74d16ce52e 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24238,6 +24238,11 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
 #define MXU_EPTN2_SS    3
 
 /* MXU operand getting pattern 'optn2' */
+#define MXU_OPTN2_PTN0  0
+#define MXU_OPTN2_PTN1  1
+#define MXU_OPTN2_PTN2  2
+#define MXU_OPTN2_PTN3  3
+/* alternative naming scheme for 'optn2' */
 #define MXU_OPTN2_WW    0
 #define MXU_OPTN2_LW    1
 #define MXU_OPTN2_HW    2
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 3/6] target/mips: MXU: Improve textual description
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions Aleksandar Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 2/6] target/mips: MXU: Add generic naming for optn2 constants Aleksandar Markovic
@ 2018-12-17 20:04 ` Aleksandar Markovic
  2018-12-18 14:30   ` Stefan Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 4/6] target/mips: MXU: Add handlers for logic instructions Aleksandar Markovic
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Improve textual description of MXU extension. These are mostly
comment formatting changes.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 74 ++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 30 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 74d16ce52e..e3a5a73e59 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1399,10 +1399,12 @@ enum {
 
 
 /*
- *    AN OVERVIEW OF MXU EXTENSION INSTRUCTION SET
- *    ============================================
  *
- * MXU (full name: MIPS eXtension/enhanced Unit) is an SIMD extension of MIPS32
+ *       AN OVERVIEW OF MXU EXTENSION INSTRUCTION SET
+ *       ============================================
+ *
+ *
+ * MXU (full name: MIPS eXtension/enhanced Unit) is a SIMD extension of MIPS32
  * instructions set. It is designed to fit the needs of signal, graphical and
  * video processing applications. MXU instruction set is used in Xburst family
  * of microprocessors by Ingenic.
@@ -1410,39 +1412,31 @@ enum {
  * MXU unit contains 17 registers called X0-X16. X0 is always zero, and X16 is
  * the control register.
  *
- * The notation used in MXU assembler mnemonics
- * --------------------------------------------
  *
- *  Registers:
+ *     The notation used in MXU assembler mnemonics
+ *     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  Register operands:
  *
  *   XRa, XRb, XRc, XRd - MXU registers
  *   Rb, Rc, Rd, Rs, Rt - general purpose MIPS registers
  *
- *  Subfields:
+ *  Non-register operands:
  *
- *   aptn1              - 1-bit accumulate add/subtract pattern
- *   aptn2              - 2-bit accumulate add/subtract pattern
- *   eptn2              - 2-bit execute add/subtract pattern
- *   optn2              - 2-bit operand pattern
- *   optn3              - 3-bit operand pattern
- *   sft4               - 4-bit shift amount
- *   strd2              - 2-bit stride amount
+ *   aptn1 - 1-bit accumulate add/subtract pattern
+ *   aptn2 - 2-bit accumulate add/subtract pattern
+ *   eptn2 - 2-bit execute add/subtract pattern
+ *   optn2 - 2-bit operand pattern
+ *   optn3 - 3-bit operand pattern
+ *   sft4  - 4-bit shift amount
+ *   strd2 - 2-bit stride amount
  *
  *  Prefixes:
  *
- *   <Operation parallel level><Operand size>
- *     S                         32
- *     D                         16
- *     Q                          8
- *
- *  Suffixes:
- *
- *   E - Expand results
- *   F - Fixed point multiplication
- *   L - Low part result
- *   R - Doing rounding
- *   V - Variable instead of immediate
- *   W - Combine above L and V
+ *   Level of parallelism:                Operand size:
+ *    S - single operation at a time       32 - word
+ *    D - two operations in parallel       16 - half word
+ *    Q - four operations in parallel       8 - byte
  *
  *  Operations:
  *
@@ -1486,6 +1480,19 @@ enum {
  *   SCOP  - Calculate x’s scope (-1, means x<0; 0, means x==0; 1, means x>0)
  *   XOR   - Logical bitwise 'exclusive or' operation
  *
+ *  Suffixes:
+ *
+ *   E - Expand results
+ *   F - Fixed point multiplication
+ *   L - Low part result
+ *   R - Doing rounding
+ *   V - Variable instead of immediate
+ *   W - Combine above L and V
+ *
+ *
+ *     The list of MXU instructions grouped by functionality
+ *     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
  * Load/Store instructions           Multiplication instructions
  * -----------------------           ---------------------------
  *
@@ -1563,6 +1570,13 @@ enum {
  *  Q16SAT XRa, XRb, XRc              S32I2M XRa, Rb
  *
  *
+ *     The opcode organization of MXU instructions
+ *     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * The bits 31..26 of all MXU instructions are equal to 0x1C (also referred
+ * as opcode SPECIAL2 in the base MIPS ISA). The organization and meaning of
+ * other bits up to the instruction level is as follows:
+ *
  *              bits
  *             05..00
  *
@@ -1700,7 +1714,7 @@ enum {
  *          │                            ├─ 010 ─ OPC_MXU_D16MOVZ
  *          │                            ├─ 011 ─ OPC_MXU_D16MOVN
  *          │                            ├─ 100 ─ OPC_MXU_S32MOVZ
- *          │                            └─ 101 ─ OPC_MXU_S32MOV
+ *          │                            └─ 101 ─ OPC_MXU_S32MOVN
  *          │
  *          │                               23..22
  *          ├─ 111010 ─ OPC_MXU__POOL21 ─┬─ 00 ─ OPC_MXU_Q8MAC
@@ -1712,10 +1726,10 @@ enum {
  *          └─ 111111 ─ <not assigned>   (overlaps with SDBBP)
  *
  *
- *   Compiled after:
+ * Compiled after:
  *
  *   "XBurst® Instruction Set Architecture MIPS eXtension/enhanced Unit
- *   Programming Manual", Ingenic Semiconductor Co, Ltd., 2017
+ *   Programming Manual", Ingenic Semiconductor Co, Ltd., revision June 2, 2017
  */
 
 enum {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 4/6] target/mips: MXU: Add handlers for logic instructions
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
                   ` (2 preceding siblings ...)
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 3/6] target/mips: MXU: Improve textual description Aleksandar Markovic
@ 2018-12-17 20:04 ` Aleksandar Markovic
  2018-12-18 14:39   ` Stefan Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions Aleksandar Markovic
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Add translation handlers for logic MXU instructions.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 182 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 170 insertions(+), 12 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index e3a5a73e59..c74a831a17 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24649,6 +24649,172 @@ static void gen_mxu_s32ldd_s32lddr(DisasContext *ctx)
 }
 
 
+/*
+ *                 MXU instruction category: logic
+ *                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *               S32NOR    S32AND    S32OR    S32XOR
+ */
+
+/*
+ *  S32NOR XRa, XRb, XRc
+ *    Update XRa with the result of logical bitwise 'nor' operation
+ *    applied to the content of XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ */
+static void gen_mxu_S32NOR(DisasContext *ctx)
+{
+    uint32_t pad, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRc == 0))) {
+        /* both operands zero registers -> just set destination to all 1s */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0xFFFFFFFF);
+    } else if (unlikely(XRb == 0)) {
+        /* XRb zero register -> just set destination to the negation of XRc */
+        tcg_gen_not_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
+    } else if (unlikely(XRc == 0)) {
+        /* XRa zero register -> just set destination to the negation of XRb */
+        tcg_gen_not_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to the negation of XRb */
+        tcg_gen_not_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else {
+        /* the most general case */
+        tcg_gen_nor_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
+    }
+}
+
+/*
+ *  S32AND XRa, XRb, XRc
+ *    Update XRa with the result of logical bitwise 'and' operation
+ *    applied to the content of XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ */
+static void gen_mxu_S32AND(DisasContext *ctx)
+{
+    uint32_t pad, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) || (XRc == 0))) {
+        /* one of operands zero register -> just set destination to all 0s */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to one of them */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else {
+        /* the most general case */
+        tcg_gen_and_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
+    }
+}
+
+/*
+ *  S32OR XRa, XRb, XRc
+ *    Update XRa with the result of logical bitwise 'or' operation
+ *    applied to the content of XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
+ *  +-----------+---------+--+--+-------+-------+-------+-----------+
+ */
+static void gen_mxu_S32OR(DisasContext *ctx)
+{
+    uint32_t pad, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRc == 0))) {
+        /* both operands zero registers -> just set destination to all 0s */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else if (unlikely(XRb == 0)) {
+        /* XRb zero register -> just set destination to the content of XRc */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
+    } else if (unlikely(XRc == 0)) {
+        /* XRc zero register -> just set destination to the content of XRb */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to the content of XRb */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else {
+        /* the most general case */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
+    }
+}
+
+/*
+ *  S32XOR XRa, XRb, XRc
+ *    Update XRa with the result of logical bitwise 'xor' operation
+ *    applied to the content of XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ */
+static void gen_mxu_S32XOR(DisasContext *ctx)
+{
+    uint32_t pad, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRc == 0))) {
+        /* both operands zero registers -> just set destination to all 0s */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else if (unlikely(XRb == 0)) {
+        /* XRb zero register -> just set destination to the content of XRc */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
+    } else if (unlikely(XRc == 0)) {
+        /* XRc zero register -> just set destination to the content of XRb */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to all 0s */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else {
+        /* the most general case */
+        tcg_gen_xor_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
+    }
+}
+
+
 /*
  * Decoding engine for MXU
  * =======================
@@ -25334,24 +25500,16 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI);
         break;
     case OPC_MXU_S32NOR:
-        /* TODO: Implement emulation of S32NOR instruction. */
-        MIPS_INVAL("OPC_MXU_S32NOR");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_S32NOR(ctx);
         break;
     case OPC_MXU_S32AND:
-        /* TODO: Implement emulation of S32AND instruction. */
-        MIPS_INVAL("OPC_MXU_S32AND");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_S32AND(ctx);
         break;
     case OPC_MXU_S32OR:
-        /* TODO: Implement emulation of S32OR instruction. */
-        MIPS_INVAL("OPC_MXU_S32OR");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_S32OR(ctx);
         break;
     case OPC_MXU_S32XOR:
-        /* TODO: Implement emulation of S32XOR instruction. */
-        MIPS_INVAL("OPC_MXU_S32XOR");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_S32XOR(ctx);
         break;
     case OPC_MXU_S32LUI:
         /* TODO: Implement emulation of S32LUI instruction. */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
                   ` (3 preceding siblings ...)
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 4/6] target/mips: MXU: Add handlers for logic instructions Aleksandar Markovic
@ 2018-12-17 20:04 ` Aleksandar Markovic
  2018-12-18 15:05   ` Stefan Markovic
  2018-12-23 17:22   ` Aleksandar Markovic
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 6/6] target/mips: MXU: Add handlers for an align instruction Aleksandar Markovic
  2018-12-27 21:27 ` [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
  6 siblings, 2 replies; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Add translation handlers for max/min MXU instructions.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 356 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 335 insertions(+), 21 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index c74a831a17..339de8c32b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24815,6 +24815,338 @@ static void gen_mxu_S32XOR(DisasContext *ctx)
 }
 
 
+/*
+ *                   MXU instruction category max/min
+ *                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *                     S32MAX     D16MAX     Q8MAX
+ *                     S32MIN     D16MIN     Q8MIN
+ */
+
+/*
+ *  S32MAX XRa, XRb, XRc
+ *    Update XRa with the maximum of signed 32-bit integers contained
+ *    in XRb and XRc.
+ *
+ *  S32MIN XRa, XRb, XRc
+ *    Update XRa with the minimum of signed 32-bit integers contained
+ *    in XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ */
+static void gen_mxu_S32MAX_S32MIN(DisasContext *ctx)
+{
+    uint32_t pad, opc, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    opc = extract32(ctx->opcode, 18, 3);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRc == 0))) {
+        /* both operands zero registers -> just set destination to zero */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else if (unlikely((XRb == 0) || (XRc == 0))) {
+        /* exactly one operand is zero register - find which one is not...*/
+        uint32_t XRx = XRb ? XRb : XRc;
+        /* ...and do max/min operation with one operand 0 */
+        if (opc == OPC_MXU_S32MAX) {
+            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
+        } else {
+            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
+        }
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to one of them */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else {
+        /* the most general case */
+        if (opc == OPC_MXU_S32MAX) {
+            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
+                                               mxu_gpr[XRc - 1]);
+        } else {
+            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
+                                               mxu_gpr[XRc - 1]);
+        }
+    }
+}
+
+/*
+ *  D16MAX
+ *    Update XRa with the 16-bit-wise maximums of signed integers
+ *    contained in XRb and XRc.
+ *
+ *  D16MIN
+ *    Update XRa with the 16-bit-wise minimums of signed integers
+ *    contained in XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ */
+static void gen_mxu_D16MAX_D16MIN(DisasContext *ctx)
+{
+    uint32_t pad, opc, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    opc = extract32(ctx->opcode, 18, 3);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRc == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRa == 0))) {
+        /* both operands zero registers -> just set destination to zero */
+        tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
+    } else if (unlikely((XRb == 0) || (XRa == 0))) {
+        /* exactly one operand is zero register - find which one is not...*/
+        uint32_t XRx = XRb ? XRb : XRc;
+        /* ...and do half-word-wise max/min with one operand 0 */
+        TCGv_i32 t0 = tcg_temp_new();
+        TCGv_i32 t1 = tcg_const_i32(0);
+
+        /* the left half-word first */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
+        if (opc == OPC_MXU_D16MAX) {
+            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
+        } else {
+            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
+        }
+
+        /* the right half-word */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 16);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_D16MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting half-words to its original position */
+        tcg_gen_shri_i32(t0, t0, 16);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        tcg_temp_free(t1);
+        tcg_temp_free(t0);
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to one of them */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else {
+        /* the most general case */
+        TCGv_i32 t0 = tcg_temp_new();
+        TCGv_i32 t1 = tcg_temp_new();
+
+        /* the left half-word first */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFFFF0000);
+        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
+        if (opc == OPC_MXU_D16MAX) {
+            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
+        } else {
+            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
+        }
+
+        /* the right half-word */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
+        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FFFF);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 16);
+        tcg_gen_shli_i32(t1, t1, 16);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_D16MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting half-words to its original position */
+        tcg_gen_shri_i32(t0, t0, 16);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        tcg_temp_free(t1);
+        tcg_temp_free(t0);
+    }
+}
+
+/*
+ *  Q8MAX
+ *    Update XRa with the 8-bit-wise maximums of signed integers
+ *    contained in XRb and XRc.
+ *
+ *  Q8MIN
+ *    Update XRa with the 8-bit-wise minimums of signed integers
+ *    contained in XRb and XRc.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
+ *  +-----------+---------+-----+-------+-------+-------+-----------+
+ */
+static void gen_mxu_Q8MAX_Q8MIN(DisasContext *ctx)
+{
+    uint32_t pad, opc, XRc, XRb, XRa;
+
+    pad = extract32(ctx->opcode, 21, 5);
+    opc = extract32(ctx->opcode, 18, 3);
+    XRc = extract32(ctx->opcode, 14, 4);
+    XRb = extract32(ctx->opcode, 10, 4);
+    XRa = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRc == 0))) {
+        /* both operands zero registers -> just set destination to zero */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else if (unlikely((XRb == 0) || (XRc == 0))) {
+        /* exactly one operand is zero register - make it be the first...*/
+        uint32_t XRx = XRb ? XRb : XRc;
+        /* ...and do byte-wise max/min with one operand 0 */
+        TCGv_i32 t0 = tcg_temp_new();
+        TCGv_i32 t1 = tcg_const_i32(0);
+
+        /* the leftmost byte (byte 3) first */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFF000000);
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
+        } else {
+            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
+        }
+
+        /* byte 2 */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x00FF0000);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 8);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting byte to its original position */
+        tcg_gen_shri_i32(t0, t0, 8);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        /* byte 1 */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FF00);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 16);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting byte to its original position */
+        tcg_gen_shri_i32(t0, t0, 24);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        /* byte 0 */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x000000FF);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 24);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting byte to its original position */
+        tcg_gen_shri_i32(t0, t0, 8);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        tcg_temp_free(t1);
+        tcg_temp_free(t0);
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just set destination to one of them */
+        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+    } else {
+        /* the most general case */
+        TCGv_i32 t0 = tcg_temp_new();
+        TCGv_i32 t1 = tcg_temp_new();
+
+        /* the leftmost byte (byte 3) first */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFF000000);
+        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFF000000);
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
+        } else {
+            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
+        }
+
+        /* byte 2 */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x00FF0000);
+        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x00FF0000);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 8);
+        tcg_gen_shli_i32(t1, t1, 8);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting byte to its original position */
+        tcg_gen_shri_i32(t0, t0, 8);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        /* byte 1 */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FF00);
+        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FF00);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 16);
+        tcg_gen_shli_i32(t1, t1, 16);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting byte to its original position */
+        tcg_gen_shri_i32(t0, t0, 24);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        /* byte 0 */
+        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x000000FF);
+        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x000000FF);
+        /* move half-words to the leftmost position */
+        tcg_gen_shli_i32(t0, t0, 24);
+        tcg_gen_shli_i32(t1, t1, 24);
+        /* t0 will be max/min of t0 and t1 */
+        if (opc == OPC_MXU_Q8MAX) {
+            tcg_gen_smax_i32(t0, t0, t1);
+        } else {
+            tcg_gen_smin_i32(t0, t0, t1);
+        }
+        /* return resulting byte to its original position */
+        tcg_gen_shri_i32(t0, t0, 8);
+        /* finaly update the destination */
+        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
+
+        tcg_temp_free(t1);
+        tcg_temp_free(t0);
+    }
+}
+
+
 /*
  * Decoding engine for MXU
  * =======================
@@ -24836,34 +25168,16 @@ static void decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx)
 
     switch (opcode) {
     case OPC_MXU_S32MAX:
-        /* TODO: Implement emulation of S32MAX instruction. */
-        MIPS_INVAL("OPC_MXU_S32MAX");
-        generate_exception_end(ctx, EXCP_RI);
-        break;
     case OPC_MXU_S32MIN:
-        /* TODO: Implement emulation of S32MIN instruction. */
-        MIPS_INVAL("OPC_MXU_S32MIN");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_S32MAX_S32MIN(ctx);
         break;
     case OPC_MXU_D16MAX:
-        /* TODO: Implement emulation of D16MAX instruction. */
-        MIPS_INVAL("OPC_MXU_D16MAX");
-        generate_exception_end(ctx, EXCP_RI);
-        break;
     case OPC_MXU_D16MIN:
-        /* TODO: Implement emulation of D16MIN instruction. */
-        MIPS_INVAL("OPC_MXU_D16MIN");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_D16MAX_D16MIN(ctx);
         break;
     case OPC_MXU_Q8MAX:
-        /* TODO: Implement emulation of Q8MAX instruction. */
-        MIPS_INVAL("OPC_MXU_Q8MAX");
-        generate_exception_end(ctx, EXCP_RI);
-        break;
     case OPC_MXU_Q8MIN:
-        /* TODO: Implement emulation of Q8MIN instruction. */
-        MIPS_INVAL("OPC_MXU_Q8MIN");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_Q8MAX_Q8MIN(ctx);
         break;
     case OPC_MXU_Q8SLT:
         /* TODO: Implement emulation of Q8SLT instruction. */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 6/6] target/mips: MXU: Add handlers for an align instruction
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
                   ` (4 preceding siblings ...)
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions Aleksandar Markovic
@ 2018-12-17 20:04 ` Aleksandar Markovic
  2018-12-18 15:19   ` Stefan Markovic
  2018-12-27 21:27 ` [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
  6 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-17 20:04 UTC (permalink / raw)
  To: qemu-devel, jancraig, smarkovic, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

Add translation handler for S32ALNI MXU instruction.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c | 197 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 194 insertions(+), 3 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 339de8c32b..96905b78ac 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -25147,6 +25147,199 @@ static void gen_mxu_Q8MAX_Q8MIN(DisasContext *ctx)
 }
 
 
+/*
+ *                 MXU instruction category: align
+ *                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *                       S32ALN     S32ALNI
+ */
+
+/*
+ *  S32ALNI XRc, XRb, XRa, optn3
+ *    Arrange bytes from XRb and XRc according to one of five sets of
+ *    rules determined by optn3, and place the result in XRa.
+ *
+ *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ *  +-----------+-----+---+-----+-------+-------+-------+-----------+
+ *  |  SPECIAL2 |optn3|0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL16|
+ *  +-----------+-----+---+-----+-------+-------+-------+-----------+
+ *
+ */
+static void gen_mxu_S32ALNI(DisasContext *ctx)
+{
+    uint32_t optn3, pad, XRc, XRb, XRa;
+
+    optn3 = extract32(ctx->opcode,  23, 3);
+    pad   = extract32(ctx->opcode,  21, 2);
+    XRc   = extract32(ctx->opcode, 14, 4);
+    XRb   = extract32(ctx->opcode, 10, 4);
+    XRa   = extract32(ctx->opcode,  6, 4);
+
+    if (unlikely(pad != 0)) {
+        /* opcode padding incorrect -> do nothing */
+    } else if (unlikely(XRa == 0)) {
+        /* destination is zero register -> do nothing */
+    } else if (unlikely((XRb == 0) && (XRc == 0))) {
+        /* both operands zero registers -> just set destination to all 0s */
+        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+    } else if (unlikely(XRb == 0)) {
+        /* XRb zero register -> just appropriatelly shift XRc into XRa */
+        switch (optn3) {
+        case MXU_OPTN3_PTN0:
+            tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+            break;
+        case MXU_OPTN3_PTN1:
+        case MXU_OPTN3_PTN2:
+        case MXU_OPTN3_PTN3:
+            tcg_gen_shri_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1],
+                             8 * (4 - optn3));
+            break;
+        case MXU_OPTN3_PTN4:
+            tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
+            break;
+        }
+    } else if (unlikely(XRc == 0)) {
+        /* XRc zero register -> just appropriatelly shift XRb into XRa */
+        switch (optn3) {
+        case MXU_OPTN3_PTN0:
+            tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+            break;
+        case MXU_OPTN3_PTN1:
+        case MXU_OPTN3_PTN2:
+        case MXU_OPTN3_PTN3:
+            tcg_gen_shri_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], 8 * optn3);
+            break;
+        case MXU_OPTN3_PTN4:
+            tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
+            break;
+        }
+    } else if (unlikely(XRb == XRc)) {
+        /* both operands same -> just rotation or moving from any of them */
+        switch (optn3) {
+        case MXU_OPTN3_PTN0:
+        case MXU_OPTN3_PTN4:
+            tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+            break;
+        case MXU_OPTN3_PTN1:
+        case MXU_OPTN3_PTN2:
+        case MXU_OPTN3_PTN3:
+            tcg_gen_rotli_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], 8 * optn3);
+            break;
+        }
+    } else {
+        /* the most general case */
+        switch (optn3) {
+        case MXU_OPTN3_PTN0:
+            {
+                /*                                         */
+                /*         XRb                XRc          */
+                /*  +---------------+                      */
+                /*  | A   B   C   D |    E   F   G   H     */
+                /*  +-------+-------+                      */
+                /*          |                              */
+                /*         XRa                             */
+                /*                                         */
+
+                tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
+            }
+            break;
+        case MXU_OPTN3_PTN1:
+            {
+                /*                                         */
+                /*         XRb                 XRc         */
+                /*      +-------------------+              */
+                /*    A | B   C   D       E | F   G   H    */
+                /*      +---------+---------+              */
+                /*                |                        */
+                /*               XRa                       */
+                /*                                         */
+
+                TCGv_i32 t0 = tcg_temp_new();
+                TCGv_i32 t1 = tcg_temp_new();
+
+                tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x00FFFFFF);
+                tcg_gen_shli_i32(t0, t0, 8);
+
+                tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFF000000);
+                tcg_gen_shri_i32(t1, t1, 24);
+
+                tcg_gen_or_i32(mxu_gpr[XRa - 1], t0, t1);
+
+                tcg_temp_free(t1);
+                tcg_temp_free(t0);
+            }
+            break;
+        case MXU_OPTN3_PTN2:
+            {
+                /*                                         */
+                /*         XRb                 XRc         */
+                /*          +-------------------+          */
+                /*    A   B | C   D       E   F | G   H    */
+                /*          +---------+---------+          */
+                /*                    |                    */
+                /*                   XRa                   */
+                /*                                         */
+
+                TCGv_i32 t0 = tcg_temp_new();
+                TCGv_i32 t1 = tcg_temp_new();
+
+                tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
+                tcg_gen_shli_i32(t0, t0, 16);
+
+                tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
+                tcg_gen_shri_i32(t1, t1, 16);
+
+                tcg_gen_or_i32(mxu_gpr[XRa - 1], t0, t1);
+
+                tcg_temp_free(t1);
+                tcg_temp_free(t0);
+            }
+            break;
+        case MXU_OPTN3_PTN3:
+            {
+                /*                                         */
+                /*         XRb                 XRc         */
+                /*              +-------------------+      */
+                /*    A   B   C | D       E   F   G | H    */
+                /*              +---------+---------+      */
+                /*                        |                */
+                /*                       XRa               */
+                /*                                         */
+
+                TCGv_i32 t0 = tcg_temp_new();
+                TCGv_i32 t1 = tcg_temp_new();
+
+                tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x000000FF);
+                tcg_gen_shli_i32(t0, t0, 24);
+
+                tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFFFF00);
+                tcg_gen_shri_i32(t1, t1, 8);
+
+                tcg_gen_or_i32(mxu_gpr[XRa - 1], t0, t1);
+
+                tcg_temp_free(t1);
+                tcg_temp_free(t0);
+            }
+            break;
+        case MXU_OPTN3_PTN4:
+            {
+                /*                                         */
+                /*         XRb                 XRc         */
+                /*                     +---------------+   */
+                /*    A   B   C   D    | E   F   G   H |   */
+                /*                     +-------+-------+   */
+                /*                             |           */
+                /*                            XRa          */
+                /*                                         */
+
+                tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
+            }
+            break;
+        }
+    }
+}
+
+
 /*
  * Decoding engine for MXU
  * =======================
@@ -25809,9 +26002,7 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI);
         break;
     case OPC_MXU_S32ALNI:
-        /* TODO: Implement emulation of S32ALNI instruction. */
-        MIPS_INVAL("OPC_MXU_S32ALNI");
-        generate_exception_end(ctx, EXCP_RI);
+        gen_mxu_S32ALNI(ctx);
         break;
     case OPC_MXU_S32NOR:
         gen_mxu_S32NOR(ctx);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions Aleksandar Markovic
@ 2018-12-18 14:21   ` Stefan Markovic
  2018-12-27 17:15     ` Janeczek, Craig
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Markovic @ 2018-12-18 14:21 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Aleksandar Markovic


On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add missing opcodes and decoding engine for LXB, LXH, LXW, LXBU,
> and LXHU instructions. They were for some reason forgotten in
> previous commits. The MXU opcode list and decoding engine should
> be now complete.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 140 +++++++++++++++++++++++++++++-----------
>   1 file changed, 102 insertions(+), 38 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index e9c23a594b..e0c8d8c2f7 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1663,12 +1663,21 @@ enum {
>    *          │                               20..18
>    *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
>    *          │                            ├─ 001 ─ OPC_MXU_S32ALN
> - *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
> - *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
> - *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
> - *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
> - *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
> - *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
> + *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
> + *          │                            ├─ 011 ─ OPC_MXU_S32NOR
> + *          │                            ├─ 100 ─ OPC_MXU_S32AND
> + *          │                            ├─ 101 ─ OPC_MXU_S32OR
> + *          │                            ├─ 110 ─ OPC_MXU_S32XOR
> + *          │                            └─ 111 ─ OPC_MXU_S32LUI
> + *          │
> + *          │                               7..5
> + *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
> + *          │                            ├─ 001 ─ OPC_MXU_LXH
> + *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
> + *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
> + *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
> + *          ├─ 101100 ─ OPC_MXU_S16LDI
> + *          ├─ 101101 ─ OPC_MXU_S16SDI
>    *          ├─ 101110 ─ OPC_MXU_S32M2I
>    *          ├─ 101111 ─ OPC_MXU_S32I2M
>    *          ├─ 110000 ─ OPC_MXU_D32SLL
> @@ -1678,15 +1687,15 @@ enum {
>    *          ├─ 110100 ─ OPC_MXU_Q16SLL   ├─ 010 ─ OPC_MXU_D32SARV
>    *          ├─ 110101 ─ OPC_MXU_Q16SLR   ├─ 011 ─ OPC_MXU_Q16SLLV
>    *          │                            ├─ 100 ─ OPC_MXU_Q16SLRV
> - *          ├─ 110110 ─ OPC_MXU__POOL17 ─┴─ 101 ─ OPC_MXU_Q16SARV
> + *          ├─ 110110 ─ OPC_MXU__POOL18 ─┴─ 101 ─ OPC_MXU_Q16SARV
>    *          │
>    *          ├─ 110111 ─ OPC_MXU_Q16SAR
>    *          │                               23..22
> - *          ├─ 111000 ─ OPC_MXU__POOL18 ─┬─ 00 ─ OPC_MXU_Q8MUL
> + *          ├─ 111000 ─ OPC_MXU__POOL19 ─┬─ 00 ─ OPC_MXU_Q8MUL
>    *          │                            └─ 01 ─ OPC_MXU_Q8MULSU
>    *          │
>    *          │                               20..18
> - *          ├─ 111001 ─ OPC_MXU__POOL19 ─┬─ 000 ─ OPC_MXU_Q8MOVZ
> + *          ├─ 111001 ─ OPC_MXU__POOL20 ─┬─ 000 ─ OPC_MXU_Q8MOVZ
>    *          │                            ├─ 001 ─ OPC_MXU_Q8MOVN
>    *          │                            ├─ 010 ─ OPC_MXU_D16MOVZ
>    *          │                            ├─ 011 ─ OPC_MXU_D16MOVN
> @@ -1694,7 +1703,7 @@ enum {
>    *          │                            └─ 101 ─ OPC_MXU_S32MOV
>    *          │
>    *          │                               23..22
> - *          ├─ 111010 ─ OPC_MXU__POOL20 ─┬─ 00 ─ OPC_MXU_Q8MAC
> + *          ├─ 111010 ─ OPC_MXU__POOL21 ─┬─ 00 ─ OPC_MXU_Q8MAC
>    *          │                            └─ 10 ─ OPC_MXU_Q8MACSU
>    *          ├─ 111011 ─ OPC_MXU_Q16SCOP
>    *          ├─ 111100 ─ OPC_MXU_Q8MADL
> @@ -1750,7 +1759,7 @@ enum {
>       OPC_MXU_S8SDI    = 0x25,
>       OPC_MXU__POOL15  = 0x26,
>       OPC_MXU__POOL16  = 0x27,
> -    OPC_MXU_LXB      = 0x28,
> +    OPC_MXU__POOL17  = 0x28,
>       /* not assigned 0x29 */
>       OPC_MXU_S16LDD   = 0x2A,
>       OPC_MXU_S16STD   = 0x2B,
> @@ -1764,11 +1773,11 @@ enum {
>       OPC_MXU_D32SAR   = 0x33,
>       OPC_MXU_Q16SLL   = 0x34,
>       OPC_MXU_Q16SLR   = 0x35,
> -    OPC_MXU__POOL17  = 0x36,
> +    OPC_MXU__POOL18  = 0x36,
>       OPC_MXU_Q16SAR   = 0x37,
> -    OPC_MXU__POOL18  = 0x38,
> -    OPC_MXU__POOL19  = 0x39,
> -    OPC_MXU__POOL20  = 0x3A,
> +    OPC_MXU__POOL19  = 0x38,
> +    OPC_MXU__POOL20  = 0x39,
> +    OPC_MXU__POOL21  = 0x3A,
>       OPC_MXU_Q16SCOP  = 0x3B,
>       OPC_MXU_Q8MADL   = 0x3C,
>       OPC_MXU_S32SFL   = 0x3D,
> @@ -1940,6 +1949,17 @@ enum {
>   /*
>    * MXU pool 17
>    */
> +enum {
> +    OPC_MXU_LXB      = 0x00,
> +    OPC_MXU_LXH      = 0x01,
> +    OPC_MXU_LXW      = 0x03,
> +    OPC_MXU_LXBU     = 0x04,
> +    OPC_MXU_LXHU     = 0x05,
> +};
> +
> +/*
> + * MXU pool 18
> + */
>   enum {
>       OPC_MXU_D32SLLV  = 0x00,
>       OPC_MXU_D32SLRV  = 0x01,
> @@ -1950,7 +1970,7 @@ enum {
>   };
>   
>   /*
> - * MXU pool 18
> + * MXU pool 19
>    */
>   enum {
>       OPC_MXU_Q8MUL    = 0x00,
> @@ -1958,7 +1978,7 @@ enum {
>   };
>   
>   /*
> - * MXU pool 19
> + * MXU pool 20
>    */
>   enum {
>       OPC_MXU_Q8MOVZ   = 0x00,
> @@ -1970,7 +1990,7 @@ enum {
>   };
>   
>   /*
> - * MXU pool 20
> + * MXU pool 21
>    */
>   enum {
>       OPC_MXU_Q8MAC    = 0x00,
> @@ -25331,12 +25351,58 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
>    * Decode MXU pool17
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+---------+---+---------+-----+-----------+
> + *  |  SPECIAL2 |    rs   |    rt   |0 0|    rd   |x x x|MXU__POOL15|
> + *  +-----------+---------+---------+---+---------+-----+-----------+
> + *
> + */
> +static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
> +{
> +    uint32_t opcode = extract32(ctx->opcode, 6, 2);
> +
> +    switch (opcode) {
> +    case OPC_MXU_LXW:
> +        /* TODO: Implement emulation of LXW instruction. */
> +        MIPS_INVAL("OPC_MXU_LXW");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXH:
> +        /* TODO: Implement emulation of LXH instruction. */
> +        MIPS_INVAL("OPC_MXU_LXH");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXHU:
> +        /* TODO: Implement emulation of LXHU instruction. */
> +        MIPS_INVAL("OPC_MXU_LXHU");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXB:
> +        /* TODO: Implement emulation of LXB instruction. */
> +        MIPS_INVAL("OPC_MXU_LXB");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXBU:
> +        /* TODO: Implement emulation of LXBU instruction. */
> +        MIPS_INVAL("OPC_MXU_LXBU");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    default:
> +        MIPS_INVAL("decode_opc_mxu");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +/*
> + *
> + * Decode MXU pool18
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |    rb   |x x x|  XRd  |  XRa  |0 0 0 0|MXU__POOL17|
> + *  |  SPECIAL2 |    rb   |x x x|  XRd  |  XRa  |0 0 0 0|MXU__POOL18|
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
> +static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 18, 3);
>   
> @@ -25380,15 +25446,15 @@ static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
>   
>   /*
>    *
> - * Decode MXU pool18
> + * Decode MXU pool19
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  +-----------+---+---+-------+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |0 0|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL18|
> + *  |  SPECIAL2 |0 0|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL19|
>    *  +-----------+---+---+-------+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
> +static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 22, 2);
>   
> @@ -25406,15 +25472,15 @@ static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
>   
>   /*
>    *
> - * Decode MXU pool19
> + * Decode MXU pool20
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL19|
> + *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL20|
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
> +static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext *ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 18, 3);
>   
> @@ -25458,15 +25524,15 @@ static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
>   
>   /*
>    *
> - * Decode MXU pool20
> + * Decode MXU pool21
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  +-----------+---+---+-------+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |an2|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL20|
> + *  |  SPECIAL2 |an2|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL21|
>    *  +-----------+---+---+-------+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext *ctx)
> +static void decode_opc_mxu__pool21(CPUMIPSState *env, DisasContext *ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 22, 2);
>   
> @@ -25669,10 +25735,8 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
>           case OPC_MXU__POOL16:
>               decode_opc_mxu__pool16(env, ctx);
>               break;
> -        case OPC_MXU_LXB:
> -            /* TODO: Implement emulation of LXB instruction. */
> -            MIPS_INVAL("OPC_MXU_LXB");
> -            generate_exception_end(ctx, EXCP_RI);
> +        case OPC_MXU__POOL17:
> +            decode_opc_mxu__pool17(env, ctx);
>               break;
>           case OPC_MXU_S16LDD:
>               /* TODO: Implement emulation of S16LDD instruction. */
> @@ -25724,23 +25788,23 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
>               MIPS_INVAL("OPC_MXU_Q16SLR");
>               generate_exception_end(ctx, EXCP_RI);
>               break;
> -        case OPC_MXU__POOL17:
> -            decode_opc_mxu__pool17(env, ctx);
> +        case OPC_MXU__POOL18:
> +            decode_opc_mxu__pool18(env, ctx);
>               break;
>           case OPC_MXU_Q16SAR:
>               /* TODO: Implement emulation of Q16SAR instruction. */
>               MIPS_INVAL("OPC_MXU_Q16SAR");
>               generate_exception_end(ctx, EXCP_RI);
>               break;
> -        case OPC_MXU__POOL18:
> -            decode_opc_mxu__pool18(env, ctx);
> -            break;
>           case OPC_MXU__POOL19:
>               decode_opc_mxu__pool19(env, ctx);
>               break;
>           case OPC_MXU__POOL20:
>               decode_opc_mxu__pool20(env, ctx);
>               break;
> +        case OPC_MXU__POOL21:
> +            decode_opc_mxu__pool21(env, ctx);
> +            break;
>           case OPC_MXU_Q16SCOP:
>               /* TODO: Implement emulation of Q16SCOP instruction. */
>               MIPS_INVAL("OPC_MXU_Q16SCOP");

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] target/mips: MXU: Improve textual description
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 3/6] target/mips: MXU: Improve textual description Aleksandar Markovic
@ 2018-12-18 14:30   ` Stefan Markovic
  0 siblings, 0 replies; 25+ messages in thread
From: Stefan Markovic @ 2018-12-18 14:30 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Aleksandar Markovic


On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Improve textual description of MXU extension. These are mostly
> comment formatting changes.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 74 ++++++++++++++++++++++++-----------------
>   1 file changed, 44 insertions(+), 30 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 74d16ce52e..e3a5a73e59 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1399,10 +1399,12 @@ enum {
>   
>   
>   /*
> - *    AN OVERVIEW OF MXU EXTENSION INSTRUCTION SET
> - *    ============================================
>    *
> - * MXU (full name: MIPS eXtension/enhanced Unit) is an SIMD extension of MIPS32
> + *       AN OVERVIEW OF MXU EXTENSION INSTRUCTION SET
> + *       ============================================
> + *
> + *
> + * MXU (full name: MIPS eXtension/enhanced Unit) is a SIMD extension of MIPS32
>    * instructions set. It is designed to fit the needs of signal, graphical and
>    * video processing applications. MXU instruction set is used in Xburst family
>    * of microprocessors by Ingenic.
> @@ -1410,39 +1412,31 @@ enum {
>    * MXU unit contains 17 registers called X0-X16. X0 is always zero, and X16 is
>    * the control register.
>    *
> - * The notation used in MXU assembler mnemonics
> - * --------------------------------------------
>    *
> - *  Registers:
> + *     The notation used in MXU assembler mnemonics
> + *     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  Register operands:
>    *
>    *   XRa, XRb, XRc, XRd - MXU registers
>    *   Rb, Rc, Rd, Rs, Rt - general purpose MIPS registers
>    *
> - *  Subfields:
> + *  Non-register operands:
>    *
> - *   aptn1              - 1-bit accumulate add/subtract pattern
> - *   aptn2              - 2-bit accumulate add/subtract pattern
> - *   eptn2              - 2-bit execute add/subtract pattern
> - *   optn2              - 2-bit operand pattern
> - *   optn3              - 3-bit operand pattern
> - *   sft4               - 4-bit shift amount
> - *   strd2              - 2-bit stride amount
> + *   aptn1 - 1-bit accumulate add/subtract pattern
> + *   aptn2 - 2-bit accumulate add/subtract pattern
> + *   eptn2 - 2-bit execute add/subtract pattern
> + *   optn2 - 2-bit operand pattern
> + *   optn3 - 3-bit operand pattern
> + *   sft4  - 4-bit shift amount
> + *   strd2 - 2-bit stride amount
>    *
>    *  Prefixes:
>    *
> - *   <Operation parallel level><Operand size>
> - *     S                         32
> - *     D                         16
> - *     Q                          8
> - *
> - *  Suffixes:
> - *
> - *   E - Expand results
> - *   F - Fixed point multiplication
> - *   L - Low part result
> - *   R - Doing rounding
> - *   V - Variable instead of immediate
> - *   W - Combine above L and V
> + *   Level of parallelism:                Operand size:
> + *    S - single operation at a time       32 - word
> + *    D - two operations in parallel       16 - half word
> + *    Q - four operations in parallel       8 - byte
>    *
>    *  Operations:
>    *
> @@ -1486,6 +1480,19 @@ enum {
>    *   SCOP  - Calculate x’s scope (-1, means x<0; 0, means x==0; 1, means x>0)
>    *   XOR   - Logical bitwise 'exclusive or' operation
>    *
> + *  Suffixes:
> + *
> + *   E - Expand results
> + *   F - Fixed point multiplication
> + *   L - Low part result
> + *   R - Doing rounding
> + *   V - Variable instead of immediate
> + *   W - Combine above L and V
> + *
> + *
> + *     The list of MXU instructions grouped by functionality
> + *     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
>    * Load/Store instructions           Multiplication instructions
>    * -----------------------           ---------------------------
>    *
> @@ -1563,6 +1570,13 @@ enum {
>    *  Q16SAT XRa, XRb, XRc              S32I2M XRa, Rb
>    *
>    *
> + *     The opcode organization of MXU instructions
> + *     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * The bits 31..26 of all MXU instructions are equal to 0x1C (also referred
> + * as opcode SPECIAL2 in the base MIPS ISA). The organization and meaning of
> + * other bits up to the instruction level is as follows:
> + *
>    *              bits
>    *             05..00
>    *
> @@ -1700,7 +1714,7 @@ enum {
>    *          │                            ├─ 010 ─ OPC_MXU_D16MOVZ
>    *          │                            ├─ 011 ─ OPC_MXU_D16MOVN
>    *          │                            ├─ 100 ─ OPC_MXU_S32MOVZ
> - *          │                            └─ 101 ─ OPC_MXU_S32MOV
> + *          │                            └─ 101 ─ OPC_MXU_S32MOVN
>    *          │
>    *          │                               23..22
>    *          ├─ 111010 ─ OPC_MXU__POOL21 ─┬─ 00 ─ OPC_MXU_Q8MAC
> @@ -1712,10 +1726,10 @@ enum {
>    *          └─ 111111 ─ <not assigned>   (overlaps with SDBBP)
>    *
>    *
> - *   Compiled after:
> + * Compiled after:
>    *
>    *   "XBurst® Instruction Set Architecture MIPS eXtension/enhanced Unit
> - *   Programming Manual", Ingenic Semiconductor Co, Ltd., 2017
> + *   Programming Manual", Ingenic Semiconductor Co, Ltd., revision June 2, 2017
>    */
>   
>   enum {

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] target/mips: MXU: Add handlers for logic instructions
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 4/6] target/mips: MXU: Add handlers for logic instructions Aleksandar Markovic
@ 2018-12-18 14:39   ` Stefan Markovic
  0 siblings, 0 replies; 25+ messages in thread
From: Stefan Markovic @ 2018-12-18 14:39 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Aleksandar Markovic


On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add translation handlers for logic MXU instructions.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 182 +++++++++++++++++++++++++++++++++++++---
>   1 file changed, 170 insertions(+), 12 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index e3a5a73e59..c74a831a17 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24649,6 +24649,172 @@ static void gen_mxu_s32ldd_s32lddr(DisasContext *ctx)
>   }
>   
>   
> +/*
> + *                 MXU instruction category: logic
> + *                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *               S32NOR    S32AND    S32OR    S32XOR
> + */
> +
> +/*
> + *  S32NOR XRa, XRb, XRc
> + *    Update XRa with the result of logical bitwise 'nor' operation
> + *    applied to the content of XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32NOR(DisasContext *ctx)
> +{
> +    uint32_t pad, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to all 1s */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0xFFFFFFFF);
> +    } else if (unlikely(XRb == 0)) {
> +        /* XRb zero register -> just set destination to the negation of XRc */
> +        tcg_gen_not_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
> +    } else if (unlikely(XRc == 0)) {
> +        /* XRa zero register -> just set destination to the negation of XRb */
> +        tcg_gen_not_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to the negation of XRb */
> +        tcg_gen_not_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        tcg_gen_nor_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
> +    }
> +}
> +
> +/*
> + *  S32AND XRa, XRb, XRc
> + *    Update XRa with the result of logical bitwise 'and' operation
> + *    applied to the content of XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32AND(DisasContext *ctx)
> +{
> +    uint32_t pad, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* one of operands zero register -> just set destination to all 0s */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        tcg_gen_and_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
> +    }
> +}
> +
> +/*
> + *  S32OR XRa, XRb, XRc
> + *    Update XRa with the result of logical bitwise 'or' operation
> + *    applied to the content of XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
> + *  +-----------+---------+--+--+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32OR(DisasContext *ctx)
> +{
> +    uint32_t pad, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to all 0s */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely(XRb == 0)) {
> +        /* XRb zero register -> just set destination to the content of XRc */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
> +    } else if (unlikely(XRc == 0)) {
> +        /* XRc zero register -> just set destination to the content of XRb */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to the content of XRb */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
> +    }
> +}
> +
> +/*
> + *  S32XOR XRa, XRb, XRc
> + *    Update XRa with the result of logical bitwise 'xor' operation
> + *    applied to the content of XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL16|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32XOR(DisasContext *ctx)
> +{
> +    uint32_t pad, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to all 0s */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely(XRb == 0)) {
> +        /* XRb zero register -> just set destination to the content of XRc */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
> +    } else if (unlikely(XRc == 0)) {
> +        /* XRc zero register -> just set destination to the content of XRb */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to all 0s */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else {
> +        /* the most general case */
> +        tcg_gen_xor_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
> +    }
> +}
> +
> +
>   /*
>    * Decoding engine for MXU
>    * =======================
> @@ -25334,24 +25500,16 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       case OPC_MXU_S32NOR:
> -        /* TODO: Implement emulation of S32NOR instruction. */
> -        MIPS_INVAL("OPC_MXU_S32NOR");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32NOR(ctx);
>           break;
>       case OPC_MXU_S32AND:
> -        /* TODO: Implement emulation of S32AND instruction. */
> -        MIPS_INVAL("OPC_MXU_S32AND");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32AND(ctx);
>           break;
>       case OPC_MXU_S32OR:
> -        /* TODO: Implement emulation of S32OR instruction. */
> -        MIPS_INVAL("OPC_MXU_S32OR");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32OR(ctx);
>           break;
>       case OPC_MXU_S32XOR:
> -        /* TODO: Implement emulation of S32XOR instruction. */
> -        MIPS_INVAL("OPC_MXU_S32XOR");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32XOR(ctx);
>           break;
>       case OPC_MXU_S32LUI:
>           /* TODO: Implement emulation of S32LUI instruction. */

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions Aleksandar Markovic
@ 2018-12-18 15:05   ` Stefan Markovic
  2018-12-27 17:18     ` Janeczek, Craig
  2018-12-23 17:22   ` Aleksandar Markovic
  1 sibling, 1 reply; 25+ messages in thread
From: Stefan Markovic @ 2018-12-18 15:05 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Aleksandar Markovic


On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add translation handlers for max/min MXU instructions.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 356 +++++++++++++++++++++++++++++++++++++---
>   1 file changed, 335 insertions(+), 21 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index c74a831a17..339de8c32b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24815,6 +24815,338 @@ static void gen_mxu_S32XOR(DisasContext *ctx)
>   }
>   
>   
> +/*
> + *                   MXU instruction category max/min
> + *                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *                     S32MAX     D16MAX     Q8MAX
> + *                     S32MIN     D16MIN     Q8MIN
> + */
> +
> +/*
> + *  S32MAX XRa, XRb, XRc
> + *    Update XRa with the maximum of signed 32-bit integers contained
> + *    in XRb and XRc.
> + *
> + *  S32MIN XRa, XRb, XRc
> + *    Update XRa with the minimum of signed 32-bit integers contained
> + *    in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32MAX_S32MIN(DisasContext *ctx)
> +{
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do max/min operation with one operand 0 */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        }
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        }
> +    }
> +}
> +
> +/*
> + *  D16MAX
> + *    Update XRa with the 16-bit-wise maximums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *  D16MIN
> + *    Update XRa with the 16-bit-wise minimums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_D16MAX_D16MIN(DisasContext *ctx)
> +{
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRc == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRa == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRa == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do half-word-wise max/min with one operand 0 */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_const_i32(0);
> +
> +        /* the left half-word first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* the right half-word */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting half-words to its original position */
> +        tcg_gen_shri_i32(t0, t0, 16);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_temp_new();
> +
> +        /* the left half-word first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFFFF0000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* the right half-word */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FFFF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        tcg_gen_shli_i32(t1, t1, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting half-words to its original position */
> +        tcg_gen_shri_i32(t0, t0, 16);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    }
> +}
> +
> +/*
> + *  Q8MAX
> + *    Update XRa with the 8-bit-wise maximums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *  Q8MIN
> + *    Update XRa with the 8-bit-wise minimums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_Q8MAX_Q8MIN(DisasContext *ctx)
> +{
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - make it be the first...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do byte-wise max/min with one operand 0 */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_const_i32(0);
> +
> +        /* the leftmost byte (byte 3) first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFF000000);
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* byte 2 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x00FF0000);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 8);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 1 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FF00);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 24);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 0 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x000000FF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 24);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_temp_new();
> +
> +        /* the leftmost byte (byte 3) first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFF000000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFF000000);
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* byte 2 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x00FF0000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x00FF0000);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 8);
> +        tcg_gen_shli_i32(t1, t1, 8);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 1 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FF00);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FF00);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        tcg_gen_shli_i32(t1, t1, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 24);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 0 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x000000FF);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x000000FF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 24);
> +        tcg_gen_shli_i32(t1, t1, 24);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    }
> +}
> +
> +
>   /*
>    * Decoding engine for MXU
>    * =======================
> @@ -24836,34 +25168,16 @@ static void decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx)
>   
>       switch (opcode) {
>       case OPC_MXU_S32MAX:
> -        /* TODO: Implement emulation of S32MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_S32MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>       case OPC_MXU_S32MIN:
> -        /* TODO: Implement emulation of S32MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_S32MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32MAX_S32MIN(ctx);
>           break;
>       case OPC_MXU_D16MAX:
> -        /* TODO: Implement emulation of D16MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_D16MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>       case OPC_MXU_D16MIN:
> -        /* TODO: Implement emulation of D16MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_D16MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_D16MAX_D16MIN(ctx);
>           break;
>       case OPC_MXU_Q8MAX:
> -        /* TODO: Implement emulation of Q8MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_Q8MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>       case OPC_MXU_Q8MIN:
> -        /* TODO: Implement emulation of Q8MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_Q8MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_Q8MAX_Q8MIN(ctx);
>           break;
>       case OPC_MXU_Q8SLT:
>           /* TODO: Implement emulation of Q8SLT instruction. */

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 6/6] target/mips: MXU: Add handlers for an align instruction
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 6/6] target/mips: MXU: Add handlers for an align instruction Aleksandar Markovic
@ 2018-12-18 15:19   ` Stefan Markovic
  0 siblings, 0 replies; 25+ messages in thread
From: Stefan Markovic @ 2018-12-18 15:19 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Aleksandar Markovic


On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add translation handler for S32ALNI MXU instruction.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 197 +++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 194 insertions(+), 3 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 339de8c32b..96905b78ac 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -25147,6 +25147,199 @@ static void gen_mxu_Q8MAX_Q8MIN(DisasContext *ctx)
>   }
>   
>   
> +/*
> + *                 MXU instruction category: align
> + *                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *                       S32ALN     S32ALNI
> + */
> +
> +/*
> + *  S32ALNI XRc, XRb, XRa, optn3
> + *    Arrange bytes from XRb and XRc according to one of five sets of
> + *    rules determined by optn3, and place the result in XRa.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+-----+---+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |optn3|0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL16|
> + *  +-----------+-----+---+-----+-------+-------+-------+-----------+
> + *
> + */
> +static void gen_mxu_S32ALNI(DisasContext *ctx)
> +{
> +    uint32_t optn3, pad, XRc, XRb, XRa;
> +
> +    optn3 = extract32(ctx->opcode,  23, 3);
> +    pad   = extract32(ctx->opcode,  21, 2);
> +    XRc   = extract32(ctx->opcode, 14, 4);
> +    XRb   = extract32(ctx->opcode, 10, 4);
> +    XRa   = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to all 0s */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely(XRb == 0)) {
> +        /* XRb zero register -> just appropriatelly shift XRc into XRa */
> +        switch (optn3) {
> +        case MXU_OPTN3_PTN0:
> +            tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +            break;
> +        case MXU_OPTN3_PTN1:
> +        case MXU_OPTN3_PTN2:
> +        case MXU_OPTN3_PTN3:
> +            tcg_gen_shri_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1],
> +                             8 * (4 - optn3));
> +            break;
> +        case MXU_OPTN3_PTN4:
> +            tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
> +            break;
> +        }
> +    } else if (unlikely(XRc == 0)) {
> +        /* XRc zero register -> just appropriatelly shift XRb into XRa */
> +        switch (optn3) {
> +        case MXU_OPTN3_PTN0:
> +            tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +            break;
> +        case MXU_OPTN3_PTN1:
> +        case MXU_OPTN3_PTN2:
> +        case MXU_OPTN3_PTN3:
> +            tcg_gen_shri_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], 8 * optn3);
> +            break;
> +        case MXU_OPTN3_PTN4:
> +            tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +            break;
> +        }
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just rotation or moving from any of them */
> +        switch (optn3) {
> +        case MXU_OPTN3_PTN0:
> +        case MXU_OPTN3_PTN4:
> +            tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +            break;
> +        case MXU_OPTN3_PTN1:
> +        case MXU_OPTN3_PTN2:
> +        case MXU_OPTN3_PTN3:
> +            tcg_gen_rotli_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1], 8 * optn3);
> +            break;
> +        }
> +    } else {
> +        /* the most general case */
> +        switch (optn3) {
> +        case MXU_OPTN3_PTN0:
> +            {
> +                /*                                         */
> +                /*         XRb                XRc          */
> +                /*  +---------------+                      */
> +                /*  | A   B   C   D |    E   F   G   H     */
> +                /*  +-------+-------+                      */
> +                /*          |                              */
> +                /*         XRa                             */
> +                /*                                         */
> +
> +                tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +            }
> +            break;
> +        case MXU_OPTN3_PTN1:
> +            {
> +                /*                                         */
> +                /*         XRb                 XRc         */
> +                /*      +-------------------+              */
> +                /*    A | B   C   D       E | F   G   H    */
> +                /*      +---------+---------+              */
> +                /*                |                        */
> +                /*               XRa                       */
> +                /*                                         */
> +
> +                TCGv_i32 t0 = tcg_temp_new();
> +                TCGv_i32 t1 = tcg_temp_new();
> +
> +                tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x00FFFFFF);
> +                tcg_gen_shli_i32(t0, t0, 8);
> +
> +                tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFF000000);
> +                tcg_gen_shri_i32(t1, t1, 24);
> +
> +                tcg_gen_or_i32(mxu_gpr[XRa - 1], t0, t1);
> +
> +                tcg_temp_free(t1);
> +                tcg_temp_free(t0);
> +            }
> +            break;
> +        case MXU_OPTN3_PTN2:
> +            {
> +                /*                                         */
> +                /*         XRb                 XRc         */
> +                /*          +-------------------+          */
> +                /*    A   B | C   D       E   F | G   H    */
> +                /*          +---------+---------+          */
> +                /*                    |                    */
> +                /*                   XRa                   */
> +                /*                                         */
> +
> +                TCGv_i32 t0 = tcg_temp_new();
> +                TCGv_i32 t1 = tcg_temp_new();
> +
> +                tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
> +                tcg_gen_shli_i32(t0, t0, 16);
> +
> +                tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
> +                tcg_gen_shri_i32(t1, t1, 16);
> +
> +                tcg_gen_or_i32(mxu_gpr[XRa - 1], t0, t1);
> +
> +                tcg_temp_free(t1);
> +                tcg_temp_free(t0);
> +            }
> +            break;
> +        case MXU_OPTN3_PTN3:
> +            {
> +                /*                                         */
> +                /*         XRb                 XRc         */
> +                /*              +-------------------+      */
> +                /*    A   B   C | D       E   F   G | H    */
> +                /*              +---------+---------+      */
> +                /*                        |                */
> +                /*                       XRa               */
> +                /*                                         */
> +
> +                TCGv_i32 t0 = tcg_temp_new();
> +                TCGv_i32 t1 = tcg_temp_new();
> +
> +                tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x000000FF);
> +                tcg_gen_shli_i32(t0, t0, 24);
> +
> +                tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFFFF00);
> +                tcg_gen_shri_i32(t1, t1, 8);
> +
> +                tcg_gen_or_i32(mxu_gpr[XRa - 1], t0, t1);
> +
> +                tcg_temp_free(t1);
> +                tcg_temp_free(t0);
> +            }
> +            break;
> +        case MXU_OPTN3_PTN4:
> +            {
> +                /*                                         */
> +                /*         XRb                 XRc         */
> +                /*                     +---------------+   */
> +                /*    A   B   C   D    | E   F   G   H |   */
> +                /*                     +-------+-------+   */
> +                /*                             |           */
> +                /*                            XRa          */
> +                /*                                         */
> +
> +                tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRc - 1]);
> +            }
> +            break;
> +        }
> +    }
> +}
> +
> +
>   /*
>    * Decoding engine for MXU
>    * =======================
> @@ -25809,9 +26002,7 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       case OPC_MXU_S32ALNI:
> -        /* TODO: Implement emulation of S32ALNI instruction. */
> -        MIPS_INVAL("OPC_MXU_S32ALNI");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32ALNI(ctx);
>           break;
>       case OPC_MXU_S32NOR:
>           gen_mxu_S32NOR(ctx);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions Aleksandar Markovic
  2018-12-18 15:05   ` Stefan Markovic
@ 2018-12-23 17:22   ` Aleksandar Markovic
  2018-12-25 19:34     ` Richard Henderson
  1 sibling, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-23 17:22 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: qemu-devel, Janeczek, Craig, Stefan Markovic, Aleksandar Markovic

On Mon, Dec 17, 2018 at 9:11 PM Aleksandar Markovic
<aleksandar.markovic@rt-rk.com> wrote:
>
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add translation handlers for max/min MXU instructions.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  target/mips/translate.c | 356 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 335 insertions(+), 21 deletions(-)
>

It would be shorter and cleaner if loops were used for processing bits
2, 1, 0 in Q8MAX and Q8MIN handlers, but, if there is no objection
over that, I can do it while applying, without respin of ghe whole
series.

Thanks,
Aleksandar

> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index c74a831a17..339de8c32b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24815,6 +24815,338 @@ static void gen_mxu_S32XOR(DisasContext *ctx)
>  }
>
>
> +/*
> + *                   MXU instruction category max/min
> + *                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *                     S32MAX     D16MAX     Q8MAX
> + *                     S32MIN     D16MIN     Q8MIN
> + */
> +
> +/*
> + *  S32MAX XRa, XRb, XRc
> + *    Update XRa with the maximum of signed 32-bit integers contained
> + *    in XRb and XRc.
> + *
> + *  S32MIN XRa, XRb, XRc
> + *    Update XRa with the minimum of signed 32-bit integers contained
> + *    in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32MAX_S32MIN(DisasContext *ctx)
> +{
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do max/min operation with one operand 0 */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        }
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        }
> +    }
> +}
> +
> +/*
> + *  D16MAX
> + *    Update XRa with the 16-bit-wise maximums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *  D16MIN
> + *    Update XRa with the 16-bit-wise minimums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_D16MAX_D16MIN(DisasContext *ctx)
> +{
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRc == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRa == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRa == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do half-word-wise max/min with one operand 0 */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_const_i32(0);
> +
> +        /* the left half-word first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* the right half-word */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting half-words to its original position */
> +        tcg_gen_shri_i32(t0, t0, 16);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_temp_new();
> +
> +        /* the left half-word first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFFFF0000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* the right half-word */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FFFF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        tcg_gen_shli_i32(t1, t1, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting half-words to its original position */
> +        tcg_gen_shri_i32(t0, t0, 16);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    }
> +}
> +
> +/*
> + *  Q8MAX
> + *    Update XRa with the 8-bit-wise maximums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *  Q8MIN
> + *    Update XRa with the 8-bit-wise minimums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_Q8MAX_Q8MIN(DisasContext *ctx)
> +{
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - make it be the first...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do byte-wise max/min with one operand 0 */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_const_i32(0);
> +
> +        /* the leftmost byte (byte 3) first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFF000000);
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* byte 2 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x00FF0000);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 8);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 1 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FF00);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 24);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 0 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x000000FF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 24);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_temp_new();
> +
> +        /* the leftmost byte (byte 3) first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFF000000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFF000000);
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* byte 2 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x00FF0000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x00FF0000);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 8);
> +        tcg_gen_shli_i32(t1, t1, 8);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 1 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FF00);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FF00);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        tcg_gen_shli_i32(t1, t1, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 24);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 0 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x000000FF);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x000000FF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 24);
> +        tcg_gen_shli_i32(t1, t1, 24);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    }
> +}
> +
> +
>  /*
>   * Decoding engine for MXU
>   * =======================
> @@ -24836,34 +25168,16 @@ static void decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx)
>
>      switch (opcode) {
>      case OPC_MXU_S32MAX:
> -        /* TODO: Implement emulation of S32MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_S32MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>      case OPC_MXU_S32MIN:
> -        /* TODO: Implement emulation of S32MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_S32MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32MAX_S32MIN(ctx);
>          break;
>      case OPC_MXU_D16MAX:
> -        /* TODO: Implement emulation of D16MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_D16MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>      case OPC_MXU_D16MIN:
> -        /* TODO: Implement emulation of D16MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_D16MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_D16MAX_D16MIN(ctx);
>          break;
>      case OPC_MXU_Q8MAX:
> -        /* TODO: Implement emulation of Q8MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_Q8MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>      case OPC_MXU_Q8MIN:
> -        /* TODO: Implement emulation of Q8MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_Q8MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_Q8MAX_Q8MIN(ctx);
>          break;
>      case OPC_MXU_Q8SLT:
>          /* TODO: Implement emulation of Q8SLT instruction. */
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-23 17:22   ` Aleksandar Markovic
@ 2018-12-25 19:34     ` Richard Henderson
  2021-03-15 21:26       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2018-12-25 19:34 UTC (permalink / raw)
  To: Aleksandar Markovic, Aleksandar Markovic
  Cc: Stefan Markovic, qemu-devel, Aleksandar Markovic, Janeczek, Craig

Sorry I missed the original post, but:

> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do max/min operation with one operand 0 */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        }
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);

You should not special case unlikely events, especially when ...

> +    } else {
> +        /* the most general case */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        }

... the normal case will handle those special cases just fine.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-18 14:21   ` Stefan Markovic
@ 2018-12-27 17:15     ` Janeczek, Craig
  2018-12-27 18:44       ` Aleksandar Markovic
  0 siblings, 1 reply; 25+ messages in thread
From: Janeczek, Craig @ 2018-12-27 17:15 UTC (permalink / raw)
  To: Stefan Markovic, Aleksandar Markovic, qemu-devel, Aleksandar Markovic

On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add missing opcodes and decoding engine for LXB, LXH, LXW, LXBU, and 
> LXHU instructions. They were for some reason forgotten in previous 
> commits. The MXU opcode list and decoding engine should be now 
> complete.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 140 +++++++++++++++++++++++++++++-----------
>   1 file changed, 102 insertions(+), 38 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> e9c23a594b..e0c8d8c2f7 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1663,12 +1663,21 @@ enum {
>    *          │                               20..18
>    *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
>    *          │                            ├─ 001 ─ OPC_MXU_S32ALN
> - *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
> - *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
> - *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
> - *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
> - *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
> - *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
> + *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
> + *          │                            ├─ 011 ─ OPC_MXU_S32NOR
> + *          │                            ├─ 100 ─ OPC_MXU_S32AND
> + *          │                            ├─ 101 ─ OPC_MXU_S32OR
> + *          │                            ├─ 110 ─ OPC_MXU_S32XOR
> + *          │                            └─ 111 ─ OPC_MXU_S32LUI
The opcodes for pool 16 do not look correct. I think the minor bits should look like the following.

┬─ 000 ─ OPC_MXU_D32SARW
├─ 001 ─ OPC_MXU_S32ALN
├─ 010 ─ OPC_MXU_S32ALNI
├─ 011 ─ OPC_MXU_S32LUI
├─ 100 ─ OPC_MXU_S32NOR
├─ 101 ─ OPC_MXU_S32AND
├─ 110 ─ OPC_MXU_S32OR
└─ 111 ─ OPC_MXU_S32XOR

> + *          │
> + *          │                               7..5
> + *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
> + *          │                            ├─ 001 ─ OPC_MXU_LXH
> + *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
> + *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
> + *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
> + *          ├─ 101100 ─ OPC_MXU_S16LDI
> + *          ├─ 101101 ─ OPC_MXU_S16SDI
>    *          ├─ 101110 ─ OPC_MXU_S32M2I
>    *          ├─ 101111 ─ OPC_MXU_S32I2M
>    *          ├─ 110000 ─ OPC_MXU_D32SLL
> @@ -1678,15 +1687,15 @@ enum {
>    *          ├─ 110100 ─ OPC_MXU_Q16SLL   ├─ 010 ─ OPC_MXU_D32SARV
>    *          ├─ 110101 ─ OPC_MXU_Q16SLR   ├─ 011 ─ OPC_MXU_Q16SLLV
>    *          │                            ├─ 100 ─ OPC_MXU_Q16SLRV
> - *          ├─ 110110 ─ OPC_MXU__POOL17 ─┴─ 101 ─ OPC_MXU_Q16SARV
> + *          ├─ 110110 ─ OPC_MXU__POOL18 ─┴─ 101 ─ OPC_MXU_Q16SARV
>    *          │
>    *          ├─ 110111 ─ OPC_MXU_Q16SAR
>    *          │                               23..22
> - *          ├─ 111000 ─ OPC_MXU__POOL18 ─┬─ 00 ─ OPC_MXU_Q8MUL
> + *          ├─ 111000 ─ OPC_MXU__POOL19 ─┬─ 00 ─ OPC_MXU_Q8MUL
>    *          │                            └─ 01 ─ OPC_MXU_Q8MULSU
>    *          │
>    *          │                               20..18
> - *          ├─ 111001 ─ OPC_MXU__POOL19 ─┬─ 000 ─ OPC_MXU_Q8MOVZ
> + *          ├─ 111001 ─ OPC_MXU__POOL20 ─┬─ 000 ─ OPC_MXU_Q8MOVZ
>    *          │                            ├─ 001 ─ OPC_MXU_Q8MOVN
>    *          │                            ├─ 010 ─ OPC_MXU_D16MOVZ
>    *          │                            ├─ 011 ─ OPC_MXU_D16MOVN
> @@ -1694,7 +1703,7 @@ enum {
>    *          │                            └─ 101 ─ OPC_MXU_S32MOV
>    *          │
>    *          │                               23..22
> - *          ├─ 111010 ─ OPC_MXU__POOL20 ─┬─ 00 ─ OPC_MXU_Q8MAC
> + *          ├─ 111010 ─ OPC_MXU__POOL21 ─┬─ 00 ─ OPC_MXU_Q8MAC
>    *          │                            └─ 10 ─ OPC_MXU_Q8MACSU
>    *          ├─ 111011 ─ OPC_MXU_Q16SCOP
>    *          ├─ 111100 ─ OPC_MXU_Q8MADL
> @@ -1750,7 +1759,7 @@ enum {
>       OPC_MXU_S8SDI    = 0x25,
>       OPC_MXU__POOL15  = 0x26,
>       OPC_MXU__POOL16  = 0x27,
> -    OPC_MXU_LXB      = 0x28,
> +    OPC_MXU__POOL17  = 0x28,
>       /* not assigned 0x29 */
>       OPC_MXU_S16LDD   = 0x2A,
>       OPC_MXU_S16STD   = 0x2B,
> @@ -1764,11 +1773,11 @@ enum {
>       OPC_MXU_D32SAR   = 0x33,
>       OPC_MXU_Q16SLL   = 0x34,
>       OPC_MXU_Q16SLR   = 0x35,
> -    OPC_MXU__POOL17  = 0x36,
> +    OPC_MXU__POOL18  = 0x36,
>       OPC_MXU_Q16SAR   = 0x37,
> -    OPC_MXU__POOL18  = 0x38,
> -    OPC_MXU__POOL19  = 0x39,
> -    OPC_MXU__POOL20  = 0x3A,
> +    OPC_MXU__POOL19  = 0x38,
> +    OPC_MXU__POOL20  = 0x39,
> +    OPC_MXU__POOL21  = 0x3A,
>       OPC_MXU_Q16SCOP  = 0x3B,
>       OPC_MXU_Q8MADL   = 0x3C,
>       OPC_MXU_S32SFL   = 0x3D,
> @@ -1940,6 +1949,17 @@ enum {
>   /*
>    * MXU pool 17
>    */
> +enum {
> +    OPC_MXU_LXB      = 0x00,
> +    OPC_MXU_LXH      = 0x01,
> +    OPC_MXU_LXW      = 0x03,
> +    OPC_MXU_LXBU     = 0x04,
> +    OPC_MXU_LXHU     = 0x05,
> +};
> +
> +/*
> + * MXU pool 18
> + */
>   enum {
>       OPC_MXU_D32SLLV  = 0x00,
>       OPC_MXU_D32SLRV  = 0x01,
> @@ -1950,7 +1970,7 @@ enum {
>   };
>   
>   /*
> - * MXU pool 18
> + * MXU pool 19
>    */
>   enum {
>       OPC_MXU_Q8MUL    = 0x00,
> @@ -1958,7 +1978,7 @@ enum {
>   };
>   
>   /*
> - * MXU pool 19
> + * MXU pool 20
>    */
>   enum {
>       OPC_MXU_Q8MOVZ   = 0x00,
> @@ -1970,7 +1990,7 @@ enum {
>   };
>   
>   /*
> - * MXU pool 20
> + * MXU pool 21
>    */
>   enum {
>       OPC_MXU_Q8MAC    = 0x00,
> @@ -25331,12 +25351,58 @@ static void decode_opc_mxu__pool16(CPUMIPSState *env, DisasContext *ctx)
>    * Decode MXU pool17
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+---------+---+---------+-----+-----------+
> + *  |  SPECIAL2 |    rs   |    rt   |0 0|    rd   |x x x|MXU__POOL15|
> + *  +-----------+---------+---------+---+---------+-----+-----------+
> + *
> + */
> +static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext 
> +*ctx) {
> +    uint32_t opcode = extract32(ctx->opcode, 6, 2);
> +
> +    switch (opcode) {
> +    case OPC_MXU_LXW:
> +        /* TODO: Implement emulation of LXW instruction. */
> +        MIPS_INVAL("OPC_MXU_LXW");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXH:
> +        /* TODO: Implement emulation of LXH instruction. */
> +        MIPS_INVAL("OPC_MXU_LXH");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXHU:
> +        /* TODO: Implement emulation of LXHU instruction. */
> +        MIPS_INVAL("OPC_MXU_LXHU");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXB:
> +        /* TODO: Implement emulation of LXB instruction. */
> +        MIPS_INVAL("OPC_MXU_LXB");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    case OPC_MXU_LXBU:
> +        /* TODO: Implement emulation of LXBU instruction. */
> +        MIPS_INVAL("OPC_MXU_LXBU");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    default:
> +        MIPS_INVAL("decode_opc_mxu");
> +        generate_exception_end(ctx, EXCP_RI);
> +        break;
> +    }
> +}
> +/*
> + *
> + * Decode MXU pool18
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |    rb   |x x x|  XRd  |  XRa  |0 0 0 0|MXU__POOL17|
> + *  |  SPECIAL2 |    rb   |x x x|  XRd  |  XRa  |0 0 0 0|MXU__POOL18|
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext 
> *ctx)
> +static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext 
> +*ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 18, 3);
>   
> @@ -25380,15 +25446,15 @@ static void 
> decode_opc_mxu__pool17(CPUMIPSState *env, DisasContext *ctx)
>   
>   /*
>    *
> - * Decode MXU pool18
> + * Decode MXU pool19
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  
> +-----------+---+---+-------+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |0 0|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL18|
> + *  |  SPECIAL2 |0 0|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL19|
>    *  +-----------+---+---+-------+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext 
> *ctx)
> +static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext 
> +*ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 22, 2);
>   
> @@ -25406,15 +25472,15 @@ static void 
> decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx)
>   
>   /*
>    *
> - * Decode MXU pool19
> + * Decode MXU pool20
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  
> +-----------+---------+-----+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL19|
> + *  |  SPECIAL2 |0 0 0 0 0|x x x|  XRc  |  XRb  |  XRa  |MXU__POOL20|
>    *  +-----------+---------+-----+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext 
> *ctx)
> +static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext 
> +*ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 18, 3);
>   
> @@ -25458,15 +25524,15 @@ static void 
> decode_opc_mxu__pool19(CPUMIPSState *env, DisasContext *ctx)
>   
>   /*
>    *
> - * Decode MXU pool20
> + * Decode MXU pool21
>    *
>    *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>    *  
> +-----------+---+---+-------+-------+-------+-------+-----------+
> - *  |  SPECIAL2 |an2|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL20|
> + *  |  SPECIAL2 |an2|x x|  XRd  |  XRc  |  XRb  |  XRa  |MXU__POOL21|
>    *  +-----------+---+---+-------+-------+-------+-------+-----------+
>    *
>    */
> -static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext 
> *ctx)
> +static void decode_opc_mxu__pool21(CPUMIPSState *env, DisasContext 
> +*ctx)
>   {
>       uint32_t opcode = extract32(ctx->opcode, 22, 2);
>   
> @@ -25669,10 +25735,8 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
>           case OPC_MXU__POOL16:
>               decode_opc_mxu__pool16(env, ctx);
>               break;
> -        case OPC_MXU_LXB:
> -            /* TODO: Implement emulation of LXB instruction. */
> -            MIPS_INVAL("OPC_MXU_LXB");
> -            generate_exception_end(ctx, EXCP_RI);
> +        case OPC_MXU__POOL17:
> +            decode_opc_mxu__pool17(env, ctx);
>               break;
>           case OPC_MXU_S16LDD:
>               /* TODO: Implement emulation of S16LDD instruction. */ 
> @@ -25724,23 +25788,23 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx)
>               MIPS_INVAL("OPC_MXU_Q16SLR");
>               generate_exception_end(ctx, EXCP_RI);
>               break;
> -        case OPC_MXU__POOL17:
> -            decode_opc_mxu__pool17(env, ctx);
> +        case OPC_MXU__POOL18:
> +            decode_opc_mxu__pool18(env, ctx);
>               break;
>           case OPC_MXU_Q16SAR:
>               /* TODO: Implement emulation of Q16SAR instruction. */
>               MIPS_INVAL("OPC_MXU_Q16SAR");
>               generate_exception_end(ctx, EXCP_RI);
>               break;
> -        case OPC_MXU__POOL18:
> -            decode_opc_mxu__pool18(env, ctx);
> -            break;
>           case OPC_MXU__POOL19:
>               decode_opc_mxu__pool19(env, ctx);
>               break;
>           case OPC_MXU__POOL20:
>               decode_opc_mxu__pool20(env, ctx);
>               break;
> +        case OPC_MXU__POOL21:
> +            decode_opc_mxu__pool21(env, ctx);
> +            break;
>           case OPC_MXU_Q16SCOP:
>               /* TODO: Implement emulation of Q16SCOP instruction. */
>               MIPS_INVAL("OPC_MXU_Q16SCOP");

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-18 15:05   ` Stefan Markovic
@ 2018-12-27 17:18     ` Janeczek, Craig
  2018-12-27 20:14       ` Aleksandar Markovic
  0 siblings, 1 reply; 25+ messages in thread
From: Janeczek, Craig @ 2018-12-27 17:18 UTC (permalink / raw)
  To: Stefan Markovic, Aleksandar Markovic, qemu-devel, Aleksandar Markovic


On 17.12.18. 21:04, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Add translation handlers for max/min MXU instructions.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c | 356 +++++++++++++++++++++++++++++++++++++---
>   1 file changed, 335 insertions(+), 21 deletions(-)


Reviewed-by: Stefan Markovic <smarkovic@wavecomp.com>


> diff --git a/target/mips/translate.c b/target/mips/translate.c index 
> c74a831a17..339de8c32b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24815,6 +24815,338 @@ static void gen_mxu_S32XOR(DisasContext *ctx)
>   }
>   
>   
> +/*
> + *                   MXU instruction category max/min
> + *                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *                     S32MAX     D16MAX     Q8MAX
> + *                     S32MIN     D16MIN     Q8MIN
> + */
> +
> +/*
> + *  S32MAX XRa, XRb, XRc
> + *    Update XRa with the maximum of signed 32-bit integers contained
> + *    in XRb and XRc.
> + *
> + *  S32MIN XRa, XRb, XRc
> + *    Update XRa with the minimum of signed 32-bit integers contained
> + *    in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_S32MAX_S32MIN(DisasContext *ctx) {
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do max/min operation with one operand 0 */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> +        }
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        if (opc == OPC_MXU_S32MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> +                                               mxu_gpr[XRc - 1]);
> +        }
> +    }
> +}
> +
> +/*
> + *  D16MAX
> + *    Update XRa with the 16-bit-wise maximums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *  D16MIN
> + *    Update XRa with the 16-bit-wise minimums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_D16MAX_D16MIN(DisasContext *ctx) {
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRc == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRa == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRa == 0))) {
> +        /* exactly one operand is zero register - find which one is not...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do half-word-wise max/min with one operand 0 */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_const_i32(0);
> +
> +        /* the left half-word first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* the right half-word */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting half-words to its original position */
> +        tcg_gen_shri_i32(t0, t0, 16);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_temp_new();
> +
> +        /* the left half-word first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFFFF0000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* the right half-word */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FFFF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        tcg_gen_shli_i32(t1, t1, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_D16MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting half-words to its original position */
> +        tcg_gen_shri_i32(t0, t0, 16);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    }
> +}
> +
> +/*
> + *  Q8MAX
> + *    Update XRa with the 8-bit-wise maximums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *  Q8MIN
> + *    Update XRa with the 8-bit-wise minimums of signed integers
> + *    contained in XRb and XRc.
> + *
> + *   1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + *  |  SPECIAL2 |0 0 0 0 0| opc |  XRc  |  XRb  |  XRa  |MXU__POOL00|
> + *  +-----------+---------+-----+-------+-------+-------+-----------+
> + */
> +static void gen_mxu_Q8MAX_Q8MIN(DisasContext *ctx) {
> +    uint32_t pad, opc, XRc, XRb, XRa;
> +
> +    pad = extract32(ctx->opcode, 21, 5);
> +    opc = extract32(ctx->opcode, 18, 3);
> +    XRc = extract32(ctx->opcode, 14, 4);
> +    XRb = extract32(ctx->opcode, 10, 4);
> +    XRa = extract32(ctx->opcode,  6, 4);
> +
> +    if (unlikely(pad != 0)) {
> +        /* opcode padding incorrect -> do nothing */
> +    } else if (unlikely(XRa == 0)) {
> +        /* destination is zero register -> do nothing */
> +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> +        /* both operands zero registers -> just set destination to zero */
> +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> +        /* exactly one operand is zero register - make it be the first...*/
> +        uint32_t XRx = XRb ? XRb : XRc;
> +        /* ...and do byte-wise max/min with one operand 0 */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_const_i32(0);
> +
> +        /* the leftmost byte (byte 3) first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFF000000);
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* byte 2 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x00FF0000);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 8);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 1 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FF00);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 24);

Should be a shift of 16 here
tcg_gen_shri_i32(t0, t0, 16);

> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 0 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x000000FF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 24);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);

Should be a shift of 24 here
tcg_gen_shri_i32(t0, t0, 24);

> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    } else if (unlikely(XRb == XRc)) {
> +        /* both operands same -> just set destination to one of them */
> +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
> +    } else {
> +        /* the most general case */
> +        TCGv_i32 t0 = tcg_temp_new();
> +        TCGv_i32 t1 = tcg_temp_new();
> +
> +        /* the leftmost byte (byte 3) first */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFF000000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFF000000);
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> +        }
> +
> +        /* byte 2 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x00FF0000);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x00FF0000);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 8);
> +        tcg_gen_shli_i32(t1, t1, 8);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);
> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 1 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FF00);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FF00);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 16);
> +        tcg_gen_shli_i32(t1, t1, 16);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 24);


Should be a shift of 16 here
tcg_gen_shri_i32(t0, t0, 16);

> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        /* byte 0 */
> +        tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x000000FF);
> +        tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x000000FF);
> +        /* move half-words to the leftmost position */
> +        tcg_gen_shli_i32(t0, t0, 24);
> +        tcg_gen_shli_i32(t1, t1, 24);
> +        /* t0 will be max/min of t0 and t1 */
> +        if (opc == OPC_MXU_Q8MAX) {
> +            tcg_gen_smax_i32(t0, t0, t1);
> +        } else {
> +            tcg_gen_smin_i32(t0, t0, t1);
> +        }
> +        /* return resulting byte to its original position */
> +        tcg_gen_shri_i32(t0, t0, 8);

Should be a shift of 24 here
tcg_gen_shri_i32(t0, t0, 24);

> +        /* finaly update the destination */
> +        tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
> +
> +        tcg_temp_free(t1);
> +        tcg_temp_free(t0);
> +    }
> +}
> +
> +
>   /*
>    * Decoding engine for MXU
>    * =======================
> @@ -24836,34 +25168,16 @@ static void 
> decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx)
>   
>       switch (opcode) {
>       case OPC_MXU_S32MAX:
> -        /* TODO: Implement emulation of S32MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_S32MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>       case OPC_MXU_S32MIN:
> -        /* TODO: Implement emulation of S32MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_S32MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_S32MAX_S32MIN(ctx);
>           break;
>       case OPC_MXU_D16MAX:
> -        /* TODO: Implement emulation of D16MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_D16MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>       case OPC_MXU_D16MIN:
> -        /* TODO: Implement emulation of D16MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_D16MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_D16MAX_D16MIN(ctx);
>           break;
>       case OPC_MXU_Q8MAX:
> -        /* TODO: Implement emulation of Q8MAX instruction. */
> -        MIPS_INVAL("OPC_MXU_Q8MAX");
> -        generate_exception_end(ctx, EXCP_RI);
> -        break;
>       case OPC_MXU_Q8MIN:
> -        /* TODO: Implement emulation of Q8MIN instruction. */
> -        MIPS_INVAL("OPC_MXU_Q8MIN");
> -        generate_exception_end(ctx, EXCP_RI);
> +        gen_mxu_Q8MAX_Q8MIN(ctx);
>           break;
>       case OPC_MXU_Q8SLT:
>           /* TODO: Implement emulation of Q8SLT instruction. */

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-27 17:15     ` Janeczek, Craig
@ 2018-12-27 18:44       ` Aleksandar Markovic
  2018-12-27 18:50         ` Janeczek, Craig
  0 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-27 18:44 UTC (permalink / raw)
  To: Janeczek, Craig, Stefan Markovic, Aleksandar Markovic, qemu-devel

> > @@ -1663,12 +1663,21 @@ enum {
> >    *          │                               20..18
> >    *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
> >    *          │                            ├─ 001 ─ OPC_MXU_S32ALN
> > - *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
> > - *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
> > - *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
> > - *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
> > - *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
> > - *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
> > + *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
> > + *          │                            ├─ 011 ─ OPC_MXU_S32NOR
> > + *          │                            ├─ 100 ─ OPC_MXU_S32AND
> > + *          │                            ├─ 101 ─ OPC_MXU_S32OR
> > + *          │                            ├─ 110 ─ OPC_MXU_S32XOR
> > + *          │                            └─ 111 ─ OPC_MXU_S32LUI
> The opcodes for pool 16 do not look correct. I think the minor bits should look like the following.
> 
> ┬─ 000 ─ OPC_MXU_D32SARW
> ├─ 001 ─ OPC_MXU_S32ALN
> ├─ 010 ─ OPC_MXU_S32ALNI
> ├─ 011 ─ OPC_MXU_S32LUI
> ├─ 100 ─ OPC_MXU_S32NOR
> ├─ 101 ─ OPC_MXU_S32AND
> ├─ 110 ─ OPC_MXU_S32OR
> └─ 111 ─ OPC_MXU_S32XOR
> 

Hi, Craig!

My primary source for opcodes was the latest Ingenic documentation, from
Ingenic's site: (dated June 2017)

ftp://ftp.ingenic.com/SOC/M200/X1000_M200_XBurst_ISA_MXU_PM.pdf

and on page 109 there is a table in the middle of the page that contains
codes that are in agreement with what is currently in QEMU.

However, I searched more, and in a repository that seems to be derived from
Android NDK r9d binutils tree, in file

https://github.com/apportable/binutils/blob/master/opcodes/mxu-opc.c

opcodes are as you said.

I guess the definitive answer would be to involve tests on hardware, no?

Thanks for bringing this up!

Aleksandar


> > + *          │
> > + *          │                               7..5
> > + *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
> > + *          │                            ├─ 001 ─ OPC_MXU_LXH
> > + *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
> > + *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
> > + *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
> > + *          ├─ 101100 ─ OPC_MXU_S16LDI
> > + *          ├─ 101101 ─ OPC_MXU_S16SDI
> >    *          ├─ 101110 ─ OPC_MXU_S32M2I
> >    *          ├─ 101111 ─ OPC_MXU_S32I2M
> >    *          ├─ 110000 ─ OPC_MXU_D32SLL

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-27 18:44       ` Aleksandar Markovic
@ 2018-12-27 18:50         ` Janeczek, Craig
  2018-12-27 19:23           ` Aleksandar Markovic
  0 siblings, 1 reply; 25+ messages in thread
From: Janeczek, Craig @ 2018-12-27 18:50 UTC (permalink / raw)
  To: Aleksandar Markovic, Stefan Markovic, Aleksandar Markovic, qemu-devel

Yes built a binary testing these instructions and ran it against HW along with a qemu binary with your patches. The opcodes align with what I mentioned in the email.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Thursday, December 27, 2018 1:44 PM
To: Janeczek, Craig <jancraig@amazon.com>; Stefan Markovic <smarkovic@wavecomp.com>; Aleksandar Markovic <aleksandar.markovic@rt-rk.com>; qemu-devel@nongnu.org
Subject: Re: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions

> > @@ -1663,12 +1663,21 @@ enum {
> >    *          │                               20..18
> >    *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
> >    *          │                            ├─ 001 ─ OPC_MXU_S32ALN
> > - *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
> > - *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
> > - *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
> > - *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
> > - *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
> > - *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
> > + *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
> > + *          │                            ├─ 011 ─ OPC_MXU_S32NOR
> > + *          │                            ├─ 100 ─ OPC_MXU_S32AND
> > + *          │                            ├─ 101 ─ OPC_MXU_S32OR
> > + *          │                            ├─ 110 ─ OPC_MXU_S32XOR
> > + *          │                            └─ 111 ─ OPC_MXU_S32LUI
> The opcodes for pool 16 do not look correct. I think the minor bits should look like the following.
> 
> ┬─ 000 ─ OPC_MXU_D32SARW
> ├─ 001 ─ OPC_MXU_S32ALN
> ├─ 010 ─ OPC_MXU_S32ALNI
> ├─ 011 ─ OPC_MXU_S32LUI
> ├─ 100 ─ OPC_MXU_S32NOR
> ├─ 101 ─ OPC_MXU_S32AND
> ├─ 110 ─ OPC_MXU_S32OR
> └─ 111 ─ OPC_MXU_S32XOR
> 

Hi, Craig!

My primary source for opcodes was the latest Ingenic documentation, from Ingenic's site: (dated June 2017)

ftp://ftp.ingenic.com/SOC/M200/X1000_M200_XBurst_ISA_MXU_PM.pdf

and on page 109 there is a table in the middle of the page that contains codes that are in agreement with what is currently in QEMU.

However, I searched more, and in a repository that seems to be derived from Android NDK r9d binutils tree, in file

https://github.com/apportable/binutils/blob/master/opcodes/mxu-opc.c

opcodes are as you said.

I guess the definitive answer would be to involve tests on hardware, no?

Thanks for bringing this up!

Aleksandar


> > + *          │
> > + *          │                               7..5
> > + *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
> > + *          │                            ├─ 001 ─ OPC_MXU_LXH
> > + *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
> > + *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
> > + *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
> > + *          ├─ 101100 ─ OPC_MXU_S16LDI
> > + *          ├─ 101101 ─ OPC_MXU_S16SDI
> >    *          ├─ 101110 ─ OPC_MXU_S32M2I
> >    *          ├─ 101111 ─ OPC_MXU_S32I2M
> >    *          ├─ 110000 ─ OPC_MXU_D32SLL

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-27 18:50         ` Janeczek, Craig
@ 2018-12-27 19:23           ` Aleksandar Markovic
  2018-12-27 19:37             ` Janeczek, Craig
  0 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-27 19:23 UTC (permalink / raw)
  To: Janeczek, Craig, Stefan Markovic, Aleksandar Markovic, qemu-devel

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Thursday, December 27, 2018 7:50 PM
> To: Aleksandar Markovic; Stefan Markovic; Aleksandar Markovic; qemu-devel@nongnu.org
> Subject: RE: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
> 
> Yes built a binary testing these instructions and ran it against HW along with a qemu binary with your > patches. The opcodes align with what I mentioned in the email.
> 

I truly appreciate your help.

Just to clarify details, beside correcting comments, does this mean that this code segment:

/*
 * MXU pool 16
 */
enum {
    OPC_MXU_D32SARW  = 0x00,
    OPC_MXU_S32ALN   = 0x01,
    OPC_MXU_S32ALNI  = 0x02,
    OPC_MXU_S32NOR   = 0x03,
    OPC_MXU_S32AND   = 0x04,
    OPC_MXU_S32OR    = 0x05,
    OPC_MXU_S32XOR   = 0x06,
    OPC_MXU_S32LUI   = 0x07,
};

should be corrected to be:

/*
 * MXU pool 16
 */
enum {
    OPC_MXU_D32SARW  = 0x00,
    OPC_MXU_S32ALN   = 0x01,
    OPC_MXU_S32ALNI  = 0x02,
    OPC_MXU_S32LUI   = 0x03,
    OPC_MXU_S32NOR   = 0x04,
    OPC_MXU_S32AND   = 0x05,
    OPC_MXU_S32OR    = 0x06,
    OPC_MXU_S32XOR   = 0x07,
};

and all things will line up for "pool 16"?

Sincerely,
Aleksandar

> -----Original Message-----
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> Subject: Re: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
> 
> > > @@ -1663,12 +1663,21 @@ enum {
> > >    *          │                               20..18
> > >    *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
> > >    *          │                            ├─ 001 ─ OPC_MXU_S32ALN
> > > - *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
> > > - *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
> > > - *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
> > > - *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
> > > - *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
> > > - *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
> > > + *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
> > > + *          │                            ├─ 011 ─ OPC_MXU_S32NOR
> > > + *          │                            ├─ 100 ─ OPC_MXU_S32AND
> > > + *          │                            ├─ 101 ─ OPC_MXU_S32OR
> > > + *          │                            ├─ 110 ─ OPC_MXU_S32XOR
> > > + *          │                            └─ 111 ─ OPC_MXU_S32LUI
> > The opcodes for pool 16 do not look correct. I think the minor bits should look like the following.
> >
> > ┬─ 000 ─ OPC_MXU_D32SARW
> > ├─ 001 ─ OPC_MXU_S32ALN
> > ├─ 010 ─ OPC_MXU_S32ALNI
> > ├─ 011 ─ OPC_MXU_S32LUI
> > ├─ 100 ─ OPC_MXU_S32NOR
> > ├─ 101 ─ OPC_MXU_S32AND
> > ├─ 110 ─ OPC_MXU_S32OR
> > └─ 111 ─ OPC_MXU_S32XOR
> >
> 
> Hi, Craig!
> 
> My primary source for opcodes was the latest Ingenic documentation, from Ingenic's site: (dated June > 2017)
> 
> ftp://ftp.ingenic.com/SOC/M200/X1000_M200_XBurst_ISA_MXU_PM.pdf
> 
> and on page 109 there is a table in the middle of the page that contains codes that are in agreement > with what is currently in QEMU.
> 
> However, I searched more, and in a repository that seems to be derived from Android NDK r9d binutils > tree, in file
> 
> https://github.com/apportable/binutils/blob/master/opcodes/mxu-opc.c
> 
> opcodes are as you said.
> 
> I guess the definitive answer would be to involve tests on hardware, no?
> 
> Thanks for bringing this up!
> 
> Aleksandar
> 
> 
> > > + *          │
> > > + *          │                               7..5
> > > + *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
> > > + *          │                            ├─ 001 ─ OPC_MXU_LXH
> > > + *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
> > > + *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
> > > + *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
> > > + *          ├─ 101100 ─ OPC_MXU_S16LDI
> > > + *          ├─ 101101 ─ OPC_MXU_S16SDI
> > >    *          ├─ 101110 ─ OPC_MXU_S32M2I
> > >    *          ├─ 101111 ─ OPC_MXU_S32I2M
> > >    *          ├─ 110000 ─ OPC_MXU_D32SLL

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-27 19:23           ` Aleksandar Markovic
@ 2018-12-27 19:37             ` Janeczek, Craig
  2018-12-27 20:12               ` Aleksandar Markovic
  0 siblings, 1 reply; 25+ messages in thread
From: Janeczek, Craig @ 2018-12-27 19:37 UTC (permalink / raw)
  To: Aleksandar Markovic, Stefan Markovic, Aleksandar Markovic, qemu-devel

Yes, both the comment and enum should be updated.

-----Original Message-----
From: Aleksandar Markovic <amarkovic@wavecomp.com> 
Sent: Thursday, December 27, 2018 2:23 PM
To: Janeczek, Craig <jancraig@amazon.com>; Stefan Markovic <smarkovic@wavecomp.com>; Aleksandar Markovic <aleksandar.markovic@rt-rk.com>; qemu-devel@nongnu.org
Subject: Re: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions

> From: Janeczek, Craig <jancraig@amazon.com>
> Sent: Thursday, December 27, 2018 7:50 PM
> To: Aleksandar Markovic; Stefan Markovic; Aleksandar Markovic; qemu-devel@nongnu.org
> Subject: RE: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
> 
> Yes built a binary testing these instructions and ran it against HW along with a qemu binary with your > patches. The opcodes align with what I mentioned in the email.
> 

I truly appreciate your help.

Just to clarify details, beside correcting comments, does this mean that this code segment:

/*
 * MXU pool 16
 */
enum {
    OPC_MXU_D32SARW  = 0x00,
    OPC_MXU_S32ALN   = 0x01,
    OPC_MXU_S32ALNI  = 0x02,
    OPC_MXU_S32NOR   = 0x03,
    OPC_MXU_S32AND   = 0x04,
    OPC_MXU_S32OR    = 0x05,
    OPC_MXU_S32XOR   = 0x06,
    OPC_MXU_S32LUI   = 0x07,
};

should be corrected to be:

/*
 * MXU pool 16
 */
enum {
    OPC_MXU_D32SARW  = 0x00,
    OPC_MXU_S32ALN   = 0x01,
    OPC_MXU_S32ALNI  = 0x02,
    OPC_MXU_S32LUI   = 0x03,
    OPC_MXU_S32NOR   = 0x04,
    OPC_MXU_S32AND   = 0x05,
    OPC_MXU_S32OR    = 0x06,
    OPC_MXU_S32XOR   = 0x07,
};

and all things will line up for "pool 16"?

Sincerely,
Aleksandar

> -----Original Message-----
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> Subject: Re: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
> 
> > > @@ -1663,12 +1663,21 @@ enum {
> > >    *          │                               20..18
> > >    *          ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW
> > >    *          │                            ├─ 001 ─ OPC_MXU_S32ALN
> > > - *          ├─ 101000 ─ OPC_MXU_LXB      ├─ 010 ─ OPC_MXU_S32ALNI
> > > - *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_S32NOR
> > > - *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_S32AND
> > > - *          ├─ 101011 ─ OPC_MXU_S16STD   ├─ 101 ─ OPC_MXU_S32OR
> > > - *          ├─ 101100 ─ OPC_MXU_S16LDI   ├─ 110 ─ OPC_MXU_S32XOR
> > > - *          ├─ 101101 ─ OPC_MXU_S16SDI   └─ 111 ─ OPC_MXU_S32LUI
> > > + *          │                            ├─ 010 ─ OPC_MXU_S32ALNI
> > > + *          │                            ├─ 011 ─ OPC_MXU_S32NOR
> > > + *          │                            ├─ 100 ─ OPC_MXU_S32AND
> > > + *          │                            ├─ 101 ─ OPC_MXU_S32OR
> > > + *          │                            ├─ 110 ─ OPC_MXU_S32XOR
> > > + *          │                            └─ 111 ─ OPC_MXU_S32LUI
> > The opcodes for pool 16 do not look correct. I think the minor bits should look like the following.
> >
> > ┬─ 000 ─ OPC_MXU_D32SARW
> > ├─ 001 ─ OPC_MXU_S32ALN
> > ├─ 010 ─ OPC_MXU_S32ALNI
> > ├─ 011 ─ OPC_MXU_S32LUI
> > ├─ 100 ─ OPC_MXU_S32NOR
> > ├─ 101 ─ OPC_MXU_S32AND
> > ├─ 110 ─ OPC_MXU_S32OR
> > └─ 111 ─ OPC_MXU_S32XOR
> >
> 
> Hi, Craig!
> 
> My primary source for opcodes was the latest Ingenic documentation, from Ingenic's site: (dated June > 2017)
> 
> ftp://ftp.ingenic.com/SOC/M200/X1000_M200_XBurst_ISA_MXU_PM.pdf
> 
> and on page 109 there is a table in the middle of the page that contains codes that are in agreement > with what is currently in QEMU.
> 
> However, I searched more, and in a repository that seems to be derived from Android NDK r9d binutils > tree, in file
> 
> https://github.com/apportable/binutils/blob/master/opcodes/mxu-opc.c
> 
> opcodes are as you said.
> 
> I guess the definitive answer would be to involve tests on hardware, no?
> 
> Thanks for bringing this up!
> 
> Aleksandar
> 
> 
> > > + *          │
> > > + *          │                               7..5
> > > + *          ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB
> > > + *          │                            ├─ 001 ─ OPC_MXU_LXH
> > > + *          ├─ 101001 ─ <not assigned>   ├─ 011 ─ OPC_MXU_LXW
> > > + *          ├─ 101010 ─ OPC_MXU_S16LDD   ├─ 100 ─ OPC_MXU_LXBU
> > > + *          ├─ 101011 ─ OPC_MXU_S16STD   └─ 101 ─ OPC_MXU_LXHU
> > > + *          ├─ 101100 ─ OPC_MXU_S16LDI
> > > + *          ├─ 101101 ─ OPC_MXU_S16SDI
> > >    *          ├─ 101110 ─ OPC_MXU_S32M2I
> > >    *          ├─ 101111 ─ OPC_MXU_S32I2M
> > >    *          ├─ 110000 ─ OPC_MXU_D32SLL

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
  2018-12-27 19:37             ` Janeczek, Craig
@ 2018-12-27 20:12               ` Aleksandar Markovic
  0 siblings, 0 replies; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-27 20:12 UTC (permalink / raw)
  To: Janeczek, Craig, Stefan Markovic, Aleksandar Markovic, qemu-devel

> From: Janeczek, Craig <jancraig@amazon.com>
>
> Yes, both the comment and enum should be updated.

OK, this will be fixed in v2.

Thanks,
Aleksandar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-27 17:18     ` Janeczek, Craig
@ 2018-12-27 20:14       ` Aleksandar Markovic
  0 siblings, 0 replies; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-27 20:14 UTC (permalink / raw)
  To: Janeczek, Craig, Stefan Markovic, Aleksandar Markovic, qemu-devel

> From: Janeczek, Craig <jancraig@amazon.com>

> > +        /* return resulting byte to its original position */
> > +        tcg_gen_shri_i32(t0, t0, 24);
> 
> Should be a shift of 16 here
> tcg_gen_shri_i32(t0, t0, 16);

You are right, for this and the next three hints. Will be fixed in v2.

Thanks,
Aleksandar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support
  2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
                   ` (5 preceding siblings ...)
  2018-12-17 20:04 ` [Qemu-devel] [PATCH 6/6] target/mips: MXU: Add handlers for an align instruction Aleksandar Markovic
@ 2018-12-27 21:27 ` Aleksandar Markovic
  2018-12-31 14:40   ` Aleksandar Markovic
  6 siblings, 1 reply; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-27 21:27 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Stefan Markovic

> From: Aleksandar Markovic <aleksandar.markovic@rt-rk.com>
> Subject: [PATCH 0/6] target/mips: Amend MXU support
> 
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> Various updates to MXU ASE support.
> 
> Aleksandar Markovic (6):
>  target/mips: MXU: Add missing opcodes/decoding for LX* instructions
>  target/mips: MXU: Add generic naming for optn2 constants
>  target/mips: MXU: Improve textual description
>  target/mips: MXU: Add handlers for logic instructions
>  target/mips: MXU: Add handlers for max/min instructions
>  target/mips: MXU: Add handlers for an align instruction

If there is no objection, to get things going a little, I am going to integrate patches 1, 2, and 3 into next MIPS pull request (scheduled shortly), while it looks other patches need more time for review.

(patch 1 is about "pool 17", issues around "pool 16" will be covered in a separate patch in v2)

Thanks to everyone,
Aleksandar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support
  2018-12-27 21:27 ` [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
@ 2018-12-31 14:40   ` Aleksandar Markovic
  0 siblings, 0 replies; 25+ messages in thread
From: Aleksandar Markovic @ 2018-12-31 14:40 UTC (permalink / raw)
  To: Aleksandar Markovic, qemu-devel, jancraig, Stefan Markovic

> From: Aleksandar Markovic
> Subject: Re: [PATCH 0/6] target/mips: Amend MXU support
> 
> > From: Aleksandar Markovic <aleksandar.markovic@rt-rk.com>
> > Subject: [PATCH 0/6] target/mips: Amend MXU support
> >
> > From: Aleksandar Markovic <amarkovic@wavecomp.com>
> >
> > Various updates to MXU ASE support.
> >
> > Aleksandar Markovic (6):
> >  target/mips: MXU: Add missing opcodes/decoding for LX* instructions
> >  target/mips: MXU: Add generic naming for optn2 constants
> >  target/mips: MXU: Improve textual description
> >  target/mips: MXU: Add handlers for logic instructions
> >  target/mips: MXU: Add handlers for max/min instructions
> >  target/mips: MXU: Add handlers for an align instruction
> 
> If there is no objection, to get things going a little, I am going to integrate patches 1, 2, and 3 into next MIPS > pull request (scheduled shortly), while it looks other patches need more time for review.
> 
> (patch 1 is about "pool 17", issues around "pool 16" will be covered in a separate patch in v2)
> 

I was told that comparing test runs on hardware and emulation
didn't point to any issue other than those mentioned in previous
responses (by Craig). I may try to integrate remaining three
patches too (with fixes for discovered issues).

> Thanks to everyone,
> Aleksandar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions
  2018-12-25 19:34     ` Richard Henderson
@ 2021-03-15 21:26       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 21:26 UTC (permalink / raw)
  To: Richard Henderson, Janeczek, Craig
  Cc: Aleksandar Markovic, Stefan Markovic,
	qemu-devel@nongnu.org Developers, Aleksandar Markovic,
	Aleksandar Markovic

On Tue, Dec 25, 2018 at 8:35 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Sorry I missed the original post, but:
>
> > +    } else if (unlikely((XRb == 0) && (XRc == 0))) {
> > +        /* both operands zero registers -> just set destination to zero */
> > +        tcg_gen_movi_i32(mxu_gpr[XRa - 1], 0);
> > +    } else if (unlikely((XRb == 0) || (XRc == 0))) {
> > +        /* exactly one operand is zero register - find which one is not...*/
> > +        uint32_t XRx = XRb ? XRb : XRc;
> > +        /* ...and do max/min operation with one operand 0 */
> > +        if (opc == OPC_MXU_S32MAX) {
> > +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> > +        } else {
> > +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRx - 1], 0);
> > +        }
> > +    } else if (unlikely(XRb == XRc)) {
> > +        /* both operands same -> just set destination to one of them */
> > +        tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
>
> You should not special case unlikely events, especially when ...
>
> > +    } else {
> > +        /* the most general case */
> > +        if (opc == OPC_MXU_S32MAX) {
> > +            tcg_gen_smax_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> > +                                               mxu_gpr[XRc - 1]);
> > +        } else {
> > +            tcg_gen_smin_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1],
> > +                                               mxu_gpr[XRc - 1]);
> > +        }
>
> ... the normal case will handle those special cases just fine.

Also because we have now 3 Coverity CID:

 *** CID 1450831:    (OVERRUN)
in gen_mxu_D16MAX_D16MIN()
1107             TCGv_i32 t0 = tcg_temp_new();
1108             TCGv_i32 t1 = tcg_const_i32(0);
1109
1110             /* the left half-word first */
1111             tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
1112             if (opc == OPC_MXU_D16MAX) {
>>>     CID 1450831:    (OVERRUN)
>>>     Overrunning array "mxu_gpr" of 15 8-byte elements at element index 4294967295 (byte offset 34359738367) using index "XRa - 1U" (which evaluates to 4294967295).
1113                 tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);

>>>     CID 1450831:    (OVERRUN)
>>>     Overrunning array "mxu_gpr" of 15 8-byte elements at element index 4294967295 (byte offset 34359738367) using index "XRa - 1U" (which evaluates to 4294967295).
1114             } else {
1115                 tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
1116             }
1117
1118             /* the right half-word */
1119             tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);

1125             } else {
1126                 tcg_gen_smin_i32(t0, t0, t1);
1127             }
1128             /* return resulting half-words to its original position */
1129             tcg_gen_shri_i32(t0, t0, 16);
1130             /* finally update the destination */
>>>     CID 1450831:    (OVERRUN)
>>>     Overrunning array "mxu_gpr" of 15 8-byte elements at element index 4294967295 (byte offset 34359738367) using index "XRa - 1U" (which evaluates to 4294967295).
1131             tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
1132
1133             tcg_temp_free(t1);
1134             tcg_temp_free(t0);
1135         } else if (unlikely(XRb == XRc)) {
1136             /* both operands same -> just set destination to one of them */


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2021-03-15 21:27 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-17 20:04 [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
2018-12-17 20:04 ` [Qemu-devel] [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions Aleksandar Markovic
2018-12-18 14:21   ` Stefan Markovic
2018-12-27 17:15     ` Janeczek, Craig
2018-12-27 18:44       ` Aleksandar Markovic
2018-12-27 18:50         ` Janeczek, Craig
2018-12-27 19:23           ` Aleksandar Markovic
2018-12-27 19:37             ` Janeczek, Craig
2018-12-27 20:12               ` Aleksandar Markovic
2018-12-17 20:04 ` [Qemu-devel] [PATCH 2/6] target/mips: MXU: Add generic naming for optn2 constants Aleksandar Markovic
2018-12-17 20:04 ` [Qemu-devel] [PATCH 3/6] target/mips: MXU: Improve textual description Aleksandar Markovic
2018-12-18 14:30   ` Stefan Markovic
2018-12-17 20:04 ` [Qemu-devel] [PATCH 4/6] target/mips: MXU: Add handlers for logic instructions Aleksandar Markovic
2018-12-18 14:39   ` Stefan Markovic
2018-12-17 20:04 ` [Qemu-devel] [PATCH 5/6] target/mips: MXU: Add handlers for max/min instructions Aleksandar Markovic
2018-12-18 15:05   ` Stefan Markovic
2018-12-27 17:18     ` Janeczek, Craig
2018-12-27 20:14       ` Aleksandar Markovic
2018-12-23 17:22   ` Aleksandar Markovic
2018-12-25 19:34     ` Richard Henderson
2021-03-15 21:26       ` Philippe Mathieu-Daudé
2018-12-17 20:04 ` [Qemu-devel] [PATCH 6/6] target/mips: MXU: Add handlers for an align instruction Aleksandar Markovic
2018-12-18 15:19   ` Stefan Markovic
2018-12-27 21:27 ` [Qemu-devel] [PATCH 0/6] target/mips: Amend MXU support Aleksandar Markovic
2018-12-31 14:40   ` Aleksandar Markovic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.