[Qemu-devel] [PATCH 0/7] tcg/mips: Minimal R6 support

[Qemu-devel] [PATCH 0/7] tcg/mips: Minimal R6 support

[James Hogan]

This patchset adds minimal MIPS r6 host support to TCG.

The first two patches are things I've noticed along the way, and are
independent of the other patches.

Patches 3-7 add R6 awareness to the most important operations, such that
instructions removed in R6 don't get emitted. This seems to be enough to
get a MIPS Linux guest booting on a MIPSr6 host (such as QEMU or I6400).
There are no doubt other improvements that could be made to better
utilise new R6 instruction encodings, but that can wait until a later
patchset.

The changes are basically:
- Patch 4: Don't use the MIPSr5 JR encoding (r6 uses JALR with rd=zero).
- Patch 5: Don't use LO/HI registers, instead using the new
  multiply/divide encodings, which map nicely to TCG ops anyway.
- Patch 6-7: Don't use MOVN/MOVZ instructions. Instead use
  SELEQZ/SELNEZ.

James Hogan (7):
  tcg-opc.h: Simplify debug_insn_start def
  disas/mips: Add R6 jr/jr.hb to disassembler
  tcg/mips: Add use_mips32r6_instructions definition
  tcg/mips: Support r6 JR encoding
  tcg/mips: Support r6 multiply/divide encodings
  tcg/mips: Support full movcond select operation
  tcg/mips: Support r6 SEL{NE,EQ}Z instead of MOVN/MOVZ

 disas/mips.c          |  2 ++
 tcg/mips/tcg-target.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++-----
 tcg/mips/tcg-target.h | 11 +++++--
 tcg/tcg-opc.h         | 12 +++----
 4 files changed, 93 insertions(+), 18 deletions(-)

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Leon Alrae <leon.alrae@imgtec.com>
-- 
2.4.9

[Qemu-devel] [PATCH 1/7] tcg-opc.h: Simplify debug_insn_start def

[James Hogan]

We already have a TLADDR_ARGS definition, so rearrange the order
slightly and use it in the definition of debug_insn_start, instead of
having an #ifdef.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg-opc.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index 02bbf3038709..f72ce9bf195a 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -174,17 +174,13 @@ DEF(muluh_i64, 1, 2, 0, IMPL(TCG_TARGET_HAS_muluh_i64))
 DEF(mulsh_i64, 1, 2, 0, IMPL(TCG_TARGET_HAS_mulsh_i64))
 
 /* QEMU specific */
-#if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
-DEF(debug_insn_start, 0, 0, 2, TCG_OPF_NOT_PRESENT)
-#else
-DEF(debug_insn_start, 0, 0, 1, TCG_OPF_NOT_PRESENT)
-#endif
-DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_END)
-DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_END)
-
 #define TLADDR_ARGS    (TARGET_LONG_BITS <= TCG_TARGET_REG_BITS ? 1 : 2)
 #define DATA64_ARGS  (TCG_TARGET_REG_BITS == 64 ? 1 : 2)
 
+DEF(debug_insn_start, 0, 0, TLADDR_ARGS, TCG_OPF_NOT_PRESENT)
+DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_END)
+DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_END)
+
 DEF(qemu_ld_i32, 1, TLADDR_ARGS, 1,
     TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS)
 DEF(qemu_st_i32, 0, TLADDR_ARGS + 1, 1,
-- 
2.4.9

[Qemu-devel] [PATCH 2/7] disas/mips: Add R6 jr/jr.hb to disassembler

[James Hogan]

MIPS r6 encodes jr as jalr zero, and jr.hb as jalr.hb zero, so add these
encodings to the MIPS disassembly table.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Leon Alrae <leon.alrae@imgtec.com>
---
 disas/mips.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/disas/mips.c b/disas/mips.c
index 01336a83852d..2a24014f2cae 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -2420,9 +2420,11 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"hibernate","",        0x42000023, 0xffffffff,	0, 			0,		V1	},
 {"ins",     "t,r,+A,+B", 0x7c000004, 0xfc00003f, WR_t|RD_s,    		0,		I33	},
 {"jr",      "s",	0x00000008, 0xfc1fffff,	UBD|RD_s,		0,		I1	},
+{"jr",      "s",	0x00000009, 0xfc1fffff,	UBD|RD_s,		0,		I32R6	}, /* jalr */
 /* jr.hb is officially MIPS{32,64}R2, but it works on R1 as jr with
    the same hazard barrier effect.  */
 {"jr.hb",   "s",	0x00000408, 0xfc1fffff,	UBD|RD_s,		0,		I32	},
+{"jr.hb",   "s",	0x00000408, 0xfc1fffff,	UBD|RD_s,		0,		I32R6	}, /* jalr.hb */
 {"j",       "s",	0x00000008, 0xfc1fffff,	UBD|RD_s,		0,		I1	}, /* jr */
 /* SVR4 PIC code requires special handling for j, so it must be a
    macro.  */
-- 
2.4.9

[Qemu-devel] [PATCH 3/7] tcg/mips: Add use_mips32r6_instructions definition

[James Hogan]

Add definition use_mips32r6_instructions to the MIPS TCG backend which
is constant 1 when built for MIPS release 6. This will be used to decide
between pre-R6 and R6 instruction encodings.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/mips/tcg-target.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index f5ba52cacfe5..e579c10b9aaa 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -96,6 +96,13 @@ extern bool use_mips32_instructions;
 extern bool use_mips32r2_instructions;
 #endif
 
+/* MIPS32R6 instruction set detection */
+#if defined(__mips_isa_rev) && (__mips_isa_rev >= 6)
+#define use_mips32r6_instructions  1
+#else
+#define use_mips32r6_instructions  0
+#endif
+
 /* optional instructions */
 #define TCG_TARGET_HAS_div_i32          1
 #define TCG_TARGET_HAS_rem_i32          1
-- 
2.4.9

[Qemu-devel] [PATCH 4/7] tcg/mips: Support r6 JR encoding

[James Hogan]

MIPSr6 encodes JR as JALR with zero as the link register, and the pre-r6
JR encoding is removed. Update TCG to use the new encoding when built
for r6.

We still use the old encoding for pre-r6, so as not to confuse return
prediction stack hardware which may detect only particular encodings of
the return instruction.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/mips/tcg-target.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 4305af967326..0a16140b1cdb 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -288,7 +288,7 @@ typedef enum {
     OPC_SRLV     = OPC_SPECIAL | 0x06,
     OPC_ROTRV    = OPC_SPECIAL | (0x01 <<  6) | 0x06,
     OPC_SRAV     = OPC_SPECIAL | 0x07,
-    OPC_JR       = OPC_SPECIAL | 0x08,
+    OPC_JR_R5    = OPC_SPECIAL | 0x08,
     OPC_JALR     = OPC_SPECIAL | 0x09,
     OPC_MOVZ     = OPC_SPECIAL | 0x0A,
     OPC_MOVN     = OPC_SPECIAL | 0x0B,
@@ -322,6 +322,9 @@ typedef enum {
     OPC_SEH      = OPC_SPECIAL3 | 0x620,
 } MIPSInsn;
 
+/* MIPS r6 doesn't have JR, JALR should be used instead */
+#define OPC_JR (use_mips32r6_instructions ? OPC_JALR : OPC_JR_R5)
+
 /*
  * Type reg
  */
-- 
2.4.9

[Qemu-devel] [PATCH 5/7] tcg/mips: Support r6 multiply/divide encodings

[James Hogan]

MIPSr6 adds several new integer multiply, divide, and modulo
instructions, and removes several pre-r6 encodings, along with the HI/LO
registers which were the implicit operands of some of those
instructions. Update TCG to use the new instructions when built for r6.

The new instructions actually map much more directly to the TCG ops, as
they only provide a single 32-bit half of the result and in a normal
general purpose register instead of HI or LO.

The mulu2_i32 and muls2_i32 operations are no longer appropriate for r6,
so they are removed from the TCG opcode table. This is because they
would need to emit two separate host instructions anyway (for the high
and low half of the result), which TCG can arrange automatically for us
in the absense of mulu2_i32/muls2_i32 by splitting it into mul_i32 and
mul*h_i32 TCG ops.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/mips/tcg-target.c | 38 ++++++++++++++++++++++++++++++++++++++
 tcg/mips/tcg-target.h |  4 ++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 0a16140b1cdb..c447db6011ea 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -295,9 +295,17 @@ typedef enum {
     OPC_MFHI     = OPC_SPECIAL | 0x10,
     OPC_MFLO     = OPC_SPECIAL | 0x12,
     OPC_MULT     = OPC_SPECIAL | 0x18,
+    OPC_MUL_R6   = OPC_SPECIAL | (0x02 <<  6) | 0x18,
+    OPC_MUH      = OPC_SPECIAL | (0x03 <<  6) | 0x18,
     OPC_MULTU    = OPC_SPECIAL | 0x19,
+    OPC_MULU     = OPC_SPECIAL | (0x02 <<  6) | 0x19,
+    OPC_MUHU     = OPC_SPECIAL | (0x03 <<  6) | 0x19,
     OPC_DIV      = OPC_SPECIAL | 0x1A,
+    OPC_DIV_R6   = OPC_SPECIAL | (0x02 <<  6) | 0x1A,
+    OPC_MOD      = OPC_SPECIAL | (0x03 <<  6) | 0x1A,
     OPC_DIVU     = OPC_SPECIAL | 0x1B,
+    OPC_DIVU_R6  = OPC_SPECIAL | (0x02 <<  6) | 0x1B,
+    OPC_MODU     = OPC_SPECIAL | (0x03 <<  6) | 0x1B,
     OPC_ADDU     = OPC_SPECIAL | 0x21,
     OPC_SUBU     = OPC_SPECIAL | 0x23,
     OPC_AND      = OPC_SPECIAL | 0x24,
@@ -1441,6 +1449,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         goto do_binaryv;
 
     case INDEX_op_mul_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_MUL_R6, a0, a1, a2);
+            break;
+        }
         if (use_mips32_instructions) {
             tcg_out_opc_reg(s, OPC_MUL, a0, a1, a2);
             break;
@@ -1448,21 +1460,45 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         i1 = OPC_MULT, i2 = OPC_MFLO;
         goto do_hilo1;
     case INDEX_op_mulsh_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_MUH, a0, a1, a2);
+            break;
+        }
         i1 = OPC_MULT, i2 = OPC_MFHI;
         goto do_hilo1;
     case INDEX_op_muluh_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_MUHU, a0, a1, a2);
+            break;
+        }
         i1 = OPC_MULTU, i2 = OPC_MFHI;
         goto do_hilo1;
     case INDEX_op_div_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_DIV_R6, a0, a1, a2);
+            break;
+        }
         i1 = OPC_DIV, i2 = OPC_MFLO;
         goto do_hilo1;
     case INDEX_op_divu_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_DIVU_R6, a0, a1, a2);
+            break;
+        }
         i1 = OPC_DIVU, i2 = OPC_MFLO;
         goto do_hilo1;
     case INDEX_op_rem_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_MOD, a0, a1, a2);
+            break;
+        }
         i1 = OPC_DIV, i2 = OPC_MFHI;
         goto do_hilo1;
     case INDEX_op_remu_i32:
+        if (use_mips32r6_instructions) {
+            tcg_out_opc_reg(s, OPC_MODU, a0, a1, a2);
+            break;
+        }
         i1 = OPC_DIVU, i2 = OPC_MFHI;
     do_hilo1:
         tcg_out_opc_reg(s, i1, 0, a1, a2);
@@ -1595,8 +1631,10 @@ static const TCGTargetOpDef mips_op_defs[] = {
 
     { INDEX_op_add_i32, { "r", "rZ", "rJ" } },
     { INDEX_op_mul_i32, { "r", "rZ", "rZ" } },
+#if !use_mips32r6_instructions
     { INDEX_op_muls2_i32, { "r", "r", "rZ", "rZ" } },
     { INDEX_op_mulu2_i32, { "r", "r", "rZ", "rZ" } },
+#endif
     { INDEX_op_mulsh_i32, { "r", "rZ", "rZ" } },
     { INDEX_op_muluh_i32, { "r", "rZ", "rZ" } },
     { INDEX_op_div_i32, { "r", "rZ", "rZ" } },
diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index e579c10b9aaa..b1cda37b668c 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -112,8 +112,8 @@ extern bool use_mips32r2_instructions;
 #define TCG_TARGET_HAS_orc_i32          0
 #define TCG_TARGET_HAS_eqv_i32          0
 #define TCG_TARGET_HAS_nand_i32         0
-#define TCG_TARGET_HAS_mulu2_i32        1
-#define TCG_TARGET_HAS_muls2_i32        1
+#define TCG_TARGET_HAS_mulu2_i32        (!use_mips32r6_instructions)
+#define TCG_TARGET_HAS_muls2_i32        (!use_mips32r6_instructions)
 #define TCG_TARGET_HAS_muluh_i32        1
 #define TCG_TARGET_HAS_mulsh_i32        1
 
-- 
2.4.9

[Qemu-devel] [PATCH 6/7] tcg/mips: Support full movcond select operation

[James Hogan]

Adapt the MIPS movcond implementation to handle the full select
operation using a pair of MOVN/MOVZ instructions.

This allows the register alias constraint to be removed (which is what
ensured v2 == dest), and allows it to be more easily extended to support
the MIPS r6 instructions SELNEZ/SELEQZ which replace MOVN/MOVZ and
require similar logic.

For example, previously we only supported:
 movcond_i32 dest, c1, c2, v1, v2=dest, cond

With the host code:
 MOV[ZN] dest, v1, [!](c1 cond c2)

Meaning:
 if (c1 cond c2)
     dest = v1;

But now v2 doesn't have to equal dest, so we can support:
 movcond_i32 dest, c1, c2, v1, v2, cond

With the host code:
 #if dest != v1
     MOV[ZN] dest, v1, [!](c1 cond c2)
 #endif
 #if dest != v2
     MOV[NZ] dest, v1, ![!](c1 cond c2)
 #endif

Meaning:
 #if dest != v1
     if ([!](c1 cond c2))
         dest = v1;
 #endif
 #if dest != v2
     if (![!](c1 cond c2))
         dest = v2;
 #endif

Impact/benefit of this patch on TB average host size for a MIPr6 guest
kernel boot was negligible, so it was considered preferable to ifdef'ing
the constraint based on the presence of MIPS r6.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/mips/tcg-target.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index c447db6011ea..9849896bd75b 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -852,13 +852,17 @@ static void tcg_out_brcond2(TCGContext *s, TCGCond cond, TCGReg al, TCGReg ah,
 }
 
 static void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGReg ret,
-                            TCGReg c1, TCGReg c2, TCGReg v)
+                            TCGReg c1, TCGReg c2, TCGReg v1, TCGReg v2)
 {
-    MIPSInsn m_opc = OPC_MOVN;
+    MIPSInsn m_opc_t = OPC_MOVN;
+    MIPSInsn m_opc_f = OPC_MOVZ;
+    const MIPSInsn m_opc_t_inv = m_opc_f;
+    const MIPSInsn m_opc_f_inv = m_opc_t;
 
     switch (cond) {
     case TCG_COND_EQ:
-        m_opc = OPC_MOVZ;
+        m_opc_t = m_opc_t_inv;
+        m_opc_f = m_opc_f_inv;
         /* FALLTHRU */
     case TCG_COND_NE:
         if (c2 != 0) {
@@ -871,14 +875,20 @@ static void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGReg ret,
         /* Minimize code size by preferring a compare not requiring INV.  */
         if (mips_cmp_map[cond] & MIPS_CMP_INV) {
             cond = tcg_invert_cond(cond);
-            m_opc = OPC_MOVZ;
+            m_opc_t = m_opc_t_inv;
+            m_opc_f = m_opc_f_inv;
         }
         tcg_out_setcond(s, cond, TCG_TMP0, c1, c2);
         c1 = TCG_TMP0;
         break;
     }
 
-    tcg_out_opc_reg(s, m_opc, ret, v, c1);
+    if (v1 != ret) {
+        tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
+    }
+    if (v2 != ret) {
+        tcg_out_opc_reg(s, m_opc_f, ret, v2, c1);
+    }
 }
 
 static void tcg_out_call_int(TCGContext *s, tcg_insn_unit *arg, bool tail)
@@ -1575,7 +1585,7 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
 
     case INDEX_op_movcond_i32:
-        tcg_out_movcond(s, args[5], a0, a1, a2, args[3]);
+        tcg_out_movcond(s, args[5], a0, a1, a2, args[3], args[4]);
         break;
 
     case INDEX_op_setcond_i32:
@@ -1664,7 +1674,7 @@ static const TCGTargetOpDef mips_op_defs[] = {
     { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
 
     { INDEX_op_brcond_i32, { "rZ", "rZ" } },
-    { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
+    { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
     { INDEX_op_setcond_i32, { "r", "rZ", "rZ" } },
     { INDEX_op_setcond2_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
 
-- 
2.4.9

[Qemu-devel] [PATCH 7/7] tcg/mips: Support r6 SEL{NE, EQ}Z instead of MOVN/MOVZ

[James Hogan]

Extend MIPS movcond implementation to support the SELNEZ/SELEQZ
instructions introduced in MIPS r6 (where MOVN/MOVZ have been removed).

Whereas the "MOVN/MOVZ rd, rs, rt" instructions have the following
semantics:
 rd = [!]rt ? rs : rd

The "SELNEZ/SELEQZ rd, rs, rt" instructions are slightly different:
 rd = [!]rt ? rs : 0

First we ensure that if one of the movcond input values is zero that it
comes last (we can swap the input arguments if we invert the condition).
This is so that it can exactly match one of the SELNEZ/SELEQZ
instructions and avoid the need to emit the other one.

Otherwise we emit the opposite instruction first into a temporary
register, and OR that into the result:
 SELNEZ/SELEQZ  TMP1, v2, c1
 SELEQZ/SELNEZ  ret, v1, c1
 OR             ret, ret, TMP1

Which does the following:
 ret = cond ? v1 : v2

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/mips/tcg-target.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 9849896bd75b..6c0159f4a38d 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -314,6 +314,8 @@ typedef enum {
     OPC_NOR      = OPC_SPECIAL | 0x27,
     OPC_SLT      = OPC_SPECIAL | 0x2A,
     OPC_SLTU     = OPC_SPECIAL | 0x2B,
+    OPC_SELEQZ   = OPC_SPECIAL | 0x35,
+    OPC_SELNEZ   = OPC_SPECIAL | 0x37,
 
     OPC_REGIMM   = 0x01 << 26,
     OPC_BLTZ     = OPC_REGIMM | (0x00 << 16),
@@ -854,11 +856,18 @@ static void tcg_out_brcond2(TCGContext *s, TCGCond cond, TCGReg al, TCGReg ah,
 static void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGReg ret,
                             TCGReg c1, TCGReg c2, TCGReg v1, TCGReg v2)
 {
-    MIPSInsn m_opc_t = OPC_MOVN;
-    MIPSInsn m_opc_f = OPC_MOVZ;
+    MIPSInsn m_opc_t = use_mips32r6_instructions ? OPC_SELNEZ : OPC_MOVN;
+    MIPSInsn m_opc_f = use_mips32r6_instructions ? OPC_SELEQZ : OPC_MOVZ;
     const MIPSInsn m_opc_t_inv = m_opc_f;
     const MIPSInsn m_opc_f_inv = m_opc_t;
 
+    /* If one of the values is zero, put it last to match SEL*Z instructions */
+    if (use_mips32r6_instructions && v1 == 0) {
+        v1 = v2;
+        v2 = 0;
+        cond = tcg_invert_cond(cond);
+    }
+
     switch (cond) {
     case TCG_COND_EQ:
         m_opc_t = m_opc_t_inv;
@@ -883,11 +892,21 @@ static void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGReg ret,
         break;
     }
 
-    if (v1 != ret) {
+    if (use_mips32r6_instructions) {
+        if (v2 != 0) {
+            tcg_out_opc_reg(s, m_opc_f, TCG_TMP1, v2, c1);
+        }
         tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
-    }
-    if (v2 != ret) {
-        tcg_out_opc_reg(s, m_opc_f, ret, v2, c1);
+        if (v2 != 0) {
+            tcg_out_opc_reg(s, OPC_OR, ret, ret, TCG_TMP1);
+        }
+    } else {
+        if (v1 != ret) {
+            tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
+        }
+        if (v2 != ret) {
+            tcg_out_opc_reg(s, m_opc_f, ret, v2, c1);
+        }
     }
 }
 
-- 
2.4.9

Re: [Qemu-devel] [PATCH 1/7] tcg-opc.h: Simplify debug_insn_start def

[Richard Henderson]

On 10/01/2015 01:30 AM, James Hogan wrote:
> We already have a TLADDR_ARGS definition, so rearrange the order
> slightly and use it in the definition of debug_insn_start, instead of
> having an #ifdef.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Richard Henderson <rth@twiddle.net>
> ---

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 3/7] tcg/mips: Add use_mips32r6_instructions definition

[Richard Henderson]

On 10/01/2015 01:30 AM, James Hogan wrote:
> Add definition use_mips32r6_instructions to the MIPS TCG backend which
> is constant 1 when built for MIPS release 6. This will be used to decide
> between pre-R6 and R6 instruction encodings.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---


Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 4/7] tcg/mips: Support r6 JR encoding

[Richard Henderson]

On 10/01/2015 01:30 AM, James Hogan wrote:
> MIPSr6 encodes JR as JALR with zero as the link register, and the pre-r6
> JR encoding is removed. Update TCG to use the new encoding when built
> for r6.
>
> We still use the old encoding for pre-r6, so as not to confuse return
> prediction stack hardware which may detect only particular encodings of
> the return instruction.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>   tcg/mips/tcg-target.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index 4305af967326..0a16140b1cdb 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -288,7 +288,7 @@ typedef enum {
>       OPC_SRLV     = OPC_SPECIAL | 0x06,
>       OPC_ROTRV    = OPC_SPECIAL | (0x01 <<  6) | 0x06,
>       OPC_SRAV     = OPC_SPECIAL | 0x07,
> -    OPC_JR       = OPC_SPECIAL | 0x08,
> +    OPC_JR_R5    = OPC_SPECIAL | 0x08,
>       OPC_JALR     = OPC_SPECIAL | 0x09,
>       OPC_MOVZ     = OPC_SPECIAL | 0x0A,
>       OPC_MOVN     = OPC_SPECIAL | 0x0B,
> @@ -322,6 +322,9 @@ typedef enum {
>       OPC_SEH      = OPC_SPECIAL3 | 0x620,
>   } MIPSInsn;
>
> +/* MIPS r6 doesn't have JR, JALR should be used instead */
> +#define OPC_JR (use_mips32r6_instructions ? OPC_JALR : OPC_JR_R5)

This could be done in the enum instead of a define, but otherwise,


Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 6/7] tcg/mips: Support full movcond select operation

[Richard Henderson]

On 10/01/2015 01:30 AM, James Hogan wrote:
> Adapt the MIPS movcond implementation to handle the full select
> operation using a pair of MOVN/MOVZ instructions.
>
> This allows the register alias constraint to be removed (which is what
> ensured v2 == dest), and allows it to be more easily extended to support
> the MIPS r6 instructions SELNEZ/SELEQZ which replace MOVN/MOVZ and
> require similar logic.
>
> For example, previously we only supported:
>   movcond_i32 dest, c1, c2, v1, v2=dest, cond
>
> With the host code:
>   MOV[ZN] dest, v1, [!](c1 cond c2)
>
> Meaning:
>   if (c1 cond c2)
>       dest = v1;
>
> But now v2 doesn't have to equal dest, so we can support:
>   movcond_i32 dest, c1, c2, v1, v2, cond
>
> With the host code:
>   #if dest != v1
>       MOV[ZN] dest, v1, [!](c1 cond c2)
>   #endif
>   #if dest != v2
>       MOV[NZ] dest, v1, ![!](c1 cond c2)
>   #endif
>
> Meaning:
>   #if dest != v1
>       if ([!](c1 cond c2))
>           dest = v1;
>   #endif
>   #if dest != v2
>       if (![!](c1 cond c2))
>           dest = v2;
>   #endif

I don't think this is a good change.  In the case of dest != v1 && dest != v2, 
we wind up with two conditional instructions.  On most targets that I'm 
familiar with, this is more expensive than a plain move.

If r6 was conditional, as opposed to something that we must perforce know about 
at compilation time, then I'd say go ahead but split this into a normal move 
followed by a conditional move.  But since that's not the case, I don't see the 
point in changing anything at all.


r~

Re: [Qemu-devel] [PATCH 6/7] tcg/mips: Support full movcond select operation

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

Hi Richard,

On Thu, Oct 01, 2015 at 06:56:39AM +1000, Richard Henderson wrote:
> On 10/01/2015 01:30 AM, James Hogan wrote:
> > Adapt the MIPS movcond implementation to handle the full select
> > operation using a pair of MOVN/MOVZ instructions.
> >
> > This allows the register alias constraint to be removed (which is what
> > ensured v2 == dest), and allows it to be more easily extended to support
> > the MIPS r6 instructions SELNEZ/SELEQZ which replace MOVN/MOVZ and
> > require similar logic.
> >
> > For example, previously we only supported:
> >   movcond_i32 dest, c1, c2, v1, v2=dest, cond
> >
> > With the host code:
> >   MOV[ZN] dest, v1, [!](c1 cond c2)
> >
> > Meaning:
> >   if (c1 cond c2)
> >       dest = v1;
> >
> > But now v2 doesn't have to equal dest, so we can support:
> >   movcond_i32 dest, c1, c2, v1, v2, cond
> >
> > With the host code:
> >   #if dest != v1
> >       MOV[ZN] dest, v1, [!](c1 cond c2)
> >   #endif
> >   #if dest != v2
> >       MOV[NZ] dest, v1, ![!](c1 cond c2)
> >   #endif
> >
> > Meaning:
> >   #if dest != v1
> >       if ([!](c1 cond c2))
> >           dest = v1;
> >   #endif
> >   #if dest != v2
> >       if (![!](c1 cond c2))
> >           dest = v2;
> >   #endif
> 
> I don't think this is a good change.  In the case of dest != v1 && dest != v2, 
> we wind up with two conditional instructions.  On most targets that I'm 
> familiar with, this is more expensive than a plain move.
> 
> If r6 was conditional, as opposed to something that we must perforce know about 
> at compilation time, then I'd say go ahead but split this into a normal move 
> followed by a conditional move.  But since that's not the case, I don't see the 
> point in changing anything at all.

Yeh, I think you're right. This is certainly the change I was struggling
to convince myself about it being worthwhile, since it was unclear to me
just what net effect the operand aliasing constraint would have on
register allocation and the generated code.

I'll resubmit my original version without this patch. R2/R6 will just
have different constraints via an ifdef so that the R2 code doesn't have
to deal with that case (since dest will always be v2).

Thanks for the review!

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 4/7] tcg/mips: Support r6 JR encoding

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]

Hi Richard,

On Thu, Oct 01, 2015 at 06:49:11AM +1000, Richard Henderson wrote:
> On 10/01/2015 01:30 AM, James Hogan wrote:
> > MIPSr6 encodes JR as JALR with zero as the link register, and the pre-r6
> > JR encoding is removed. Update TCG to use the new encoding when built
> > for r6.
> >
> > We still use the old encoding for pre-r6, so as not to confuse return
> > prediction stack hardware which may detect only particular encodings of
> > the return instruction.
> >
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >   tcg/mips/tcg-target.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> > index 4305af967326..0a16140b1cdb 100644
> > --- a/tcg/mips/tcg-target.c
> > +++ b/tcg/mips/tcg-target.c
> > @@ -288,7 +288,7 @@ typedef enum {
> >       OPC_SRLV     = OPC_SPECIAL | 0x06,
> >       OPC_ROTRV    = OPC_SPECIAL | (0x01 <<  6) | 0x06,
> >       OPC_SRAV     = OPC_SPECIAL | 0x07,
> > -    OPC_JR       = OPC_SPECIAL | 0x08,
> > +    OPC_JR_R5    = OPC_SPECIAL | 0x08,
> >       OPC_JALR     = OPC_SPECIAL | 0x09,
> >       OPC_MOVZ     = OPC_SPECIAL | 0x0A,
> >       OPC_MOVN     = OPC_SPECIAL | 0x0B,
> > @@ -322,6 +322,9 @@ typedef enum {
> >       OPC_SEH      = OPC_SPECIAL3 | 0x620,
> >   } MIPSInsn;
> >
> > +/* MIPS r6 doesn't have JR, JALR should be used instead */
> > +#define OPC_JR (use_mips32r6_instructions ? OPC_JALR : OPC_JR_R5)
> 
> This could be done in the enum instead of a define, but otherwise,

True, I'll do that.

Thanks
James

> 
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> 
> 
> r~
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 5/7] tcg/mips: Support r6 multiply/divide encodings

[Richard Henderson]

On 10/01/2015 01:30 AM, James Hogan wrote:
>       case INDEX_op_mul_i32:
> +        if (use_mips32r6_instructions) {
> +            tcg_out_opc_reg(s, OPC_MUL_R6, a0, a1, a2);
> +            break;
> +        }
>           if (use_mips32_instructions) {
>               tcg_out_opc_reg(s, OPC_MUL, a0, a1, a2);
>               break;

I wonder if it's worth defining a common OPC_MUL as you did with OPC_JR.
Also, these columns aren't lining up.  Did you use tabs?


r~

Re: [Qemu-devel] [PATCH 5/7] tcg/mips: Support r6 multiply/divide encodings

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On Thu, Oct 01, 2015 at 09:24:17AM +1000, Richard Henderson wrote:
> On 10/01/2015 01:30 AM, James Hogan wrote:
> >       case INDEX_op_mul_i32:
> > +        if (use_mips32r6_instructions) {
> > +            tcg_out_opc_reg(s, OPC_MUL_R6, a0, a1, a2);
> > +            break;
> > +        }
> >           if (use_mips32_instructions) {
> >               tcg_out_opc_reg(s, OPC_MUL, a0, a1, a2);
> >               break;
> 
> I wonder if it's worth defining a common OPC_MUL as you did with OPC_JR.

Yes, probably would make sense for these changed instruction encodings
(rather than new instructions).

> Also, these columns aren't lining up.  Did you use tabs?

Whitespace looks okay to me. Has your email client changed it?

Thanks
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]