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

[Qemu-devel] [PATCH v3 0/6] 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-6 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 R6 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: Don't use MOVN/MOVZ instructions. Instead use
  SELEQZ/SELNEZ.


Changes in v3:
- Patch 2: Whoops. Fix jr.hb r6 encoding (Leon)
- Patch 6: Switch to using bool eqz to indicate whether to use SELEQZ /
  MOVZ instead of SELNEZ / MOVN (Richard).
- Patch 6: Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment
  to remind reader that it should be guaranteed via constraints
  (Richard).
- Add Richard's Reviewed-by to patch 2, 5, and Leon's to patch 2

Changes in v2:
- Patch 4: Turn #define into enum (Richard).
- Patch 5: Use a common OPC_MUL definition. use_mips32_instructions will
  always be 1 for MIPS r6 builds (Richard)
- Combine with patch 6 & 7 from v1, and drop functional changes to
  movcond implementation pre-r6. We now provide different constraints
  for movcond depending on presence of r6. (thanks Richard for
  feedback).
- Add Richard's Reviewed-by to patches 1, 3, 4.

James Hogan (6):
  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 r6 SEL{NE,EQ}Z instead of MOVN/MOVZ

 disas/mips.c          |  2 ++
 tcg/mips/tcg-target.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++-----
 tcg/mips/tcg-target.h | 11 +++++--
 tcg/tcg-opc.h         | 12 +++----
 4 files changed, 97 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 v3 1/6] 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>
Reviewed-by: 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

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

[Aurelien Jarno]

On 2015-10-02 13:24, 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>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/tcg-opc.h | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

Note that it will conflict with the "tcg: Rename debug_insn_start to
insn_start" patch that Richard has posted on the list, but it should
be trivial to solve it.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH v3 2/6] 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>
Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
Changes in v3:
- Whoops. Fix jr.hb r6 encoding (Leon)
---
 disas/mips.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/disas/mips.c b/disas/mips.c
index 01336a83852d..bf0bbaf86a3c 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",	0x00000409, 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

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

[Aurelien Jarno]

On 2015-10-02 13:24, James Hogan wrote:
> 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>
> Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> Changes in v3:
> - Whoops. Fix jr.hb r6 encoding (Leon)
> ---
>  disas/mips.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH v3 3/6] 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>
Reviewed-by: 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

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

[Aurelien Jarno]

On 2015-10-02 13:24, 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>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  tcg/mips/tcg-target.h | 7 +++++++
>  1 file changed, 7 insertions(+)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH v3 4/6] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
Changes in v2:
- Turn #define into enum (Richard).
---
 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..c08418c413d7 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,
@@ -320,6 +320,9 @@ typedef enum {
     OPC_WSBH     = OPC_SPECIAL3 | 0x0a0,
     OPC_SEB      = OPC_SPECIAL3 | 0x420,
     OPC_SEH      = OPC_SPECIAL3 | 0x620,
+
+    /* MIPS r6 doesn't have JR, JALR should be used instead */
+    OPC_JR       = use_mips32r6_instructions ? OPC_JALR : OPC_JR_R5,
 } MIPSInsn;
 
 /*
-- 
2.4.9

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

[Aurelien Jarno]

On 2015-10-02 13:24, 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>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> Changes in v2:
> - Turn #define into enum (Richard).
> ---
>  tcg/mips/tcg-target.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH v3 5/6] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
Changes in v2:
- Use a common OPC_MUL definition. use_mips32_instructions will always
  be 1 for MIPS r6 builds (Richard)
---
 tcg/mips/tcg-target.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 tcg/mips/tcg-target.h |  4 ++--
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index c08418c413d7..a937b1475d04 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,
@@ -312,7 +320,7 @@ typedef enum {
     OPC_BGEZ     = OPC_REGIMM | (0x01 << 16),
 
     OPC_SPECIAL2 = 0x1c << 26,
-    OPC_MUL      = OPC_SPECIAL2 | 0x002,
+    OPC_MUL_R5   = OPC_SPECIAL2 | 0x002,
 
     OPC_SPECIAL3 = 0x1f << 26,
     OPC_EXT      = OPC_SPECIAL3 | 0x000,
@@ -323,6 +331,12 @@ typedef enum {
 
     /* MIPS r6 doesn't have JR, JALR should be used instead */
     OPC_JR       = use_mips32r6_instructions ? OPC_JALR : OPC_JR_R5,
+
+    /*
+     * MIPS r6 replaces MUL with an alternative encoding which is
+     * backwards-compatible at the assembly level.
+     */
+    OPC_MUL      = use_mips32r6_instructions ? OPC_MUL_R6 : OPC_MUL_R5,
 } MIPSInsn;
 
 /*
@@ -1448,21 +1462,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 +1633,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

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

[Aurelien Jarno]

On 2015-10-02 13:24, James Hogan wrote:
> 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>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
> Changes in v2:
> - Use a common OPC_MUL definition. use_mips32_instructions will always
>   be 1 for MIPS r6 builds (Richard)
> ---
>  tcg/mips/tcg-target.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>  tcg/mips/tcg-target.h |  4 ++--
>  2 files changed, 43 insertions(+), 3 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH v3 6/6] 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>
---
Changes in v3:
- Switch to using bool eqz to indicate whether to use SELEQZ / MOVZ
  instead of SELNEZ / MOVN (Richard).
- Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment to remind
  reader that it should be guaranteed via constraints (Richard).

Changes in v2:
- Combine with patch 6 from v1, and drop functional changes to movcond
  implementation pre-r6. We now provide different constraints for
  movcond depending on presence of r6. (thanks Richard for feedback).
---
 tcg/mips/tcg-target.c | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index a937b1475d04..eff498991096 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),
@@ -858,13 +860,20 @@ 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;
+    bool eqz = false;
+
+    /* 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 = OPC_MOVZ;
+        eqz = true;
         /* FALLTHRU */
     case TCG_COND_NE:
         if (c2 != 0) {
@@ -877,14 +886,32 @@ 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;
+            eqz = true;
         }
         tcg_out_setcond(s, cond, TCG_TMP0, c1, c2);
         c1 = TCG_TMP0;
         break;
     }
 
-    tcg_out_opc_reg(s, m_opc, ret, v, c1);
+    if (use_mips32r6_instructions) {
+        MIPSInsn m_opc_t = eqz ? OPC_SELEQZ : OPC_SELNEZ;
+        MIPSInsn m_opc_f = eqz ? OPC_SELNEZ : OPC_SELEQZ;
+
+        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 != 0) {
+            tcg_out_opc_reg(s, OPC_OR, ret, ret, TCG_TMP1);
+        }
+    } else {
+        MIPSInsn m_opc = eqz ? OPC_MOVZ : OPC_MOVN;
+
+        tcg_out_opc_reg(s, m_opc, ret, v1, c1);
+
+        /* This should be guaranteed via constraints */
+        tcg_debug_assert(v2 == ret);
+    }
 }
 
 static void tcg_out_call_int(TCGContext *s, tcg_insn_unit *arg, bool tail)
@@ -1577,7 +1604,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:
@@ -1666,7 +1693,11 @@ static const TCGTargetOpDef mips_op_defs[] = {
     { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
 
     { INDEX_op_brcond_i32, { "rZ", "rZ" } },
+#if !use_mips32r6_instructions
     { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
+#else
+    { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
+#endif
     { INDEX_op_setcond_i32, { "r", "rZ", "rZ" } },
     { INDEX_op_setcond2_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
 
-- 
2.4.9

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

[Richard Henderson]

On 10/02/2015 10:24 PM, James Hogan wrote:
> 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>

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


>      { INDEX_op_brcond_i32, { "rZ", "rZ" } },
> +#if !use_mips32r6_instructions
>      { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
> +#else
> +    { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
> +#endif


The only thing I'd change is preferring positive tests to negative ones.  So 
swap the order of these lines, and the sense of the #if.

Leon, do you want to take this as a mips maintainer, or shall I as tcg maintainer?


r~

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

[James Hogan]

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

On Wed, Oct 07, 2015 at 08:46:30PM +1100, Richard Henderson wrote:
> On 10/02/2015 10:24 PM, James Hogan wrote:
> > 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>
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks for the reviewing!

> 
> 
> >      { INDEX_op_brcond_i32, { "rZ", "rZ" } },
> > +#if !use_mips32r6_instructions
> >      { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
> > +#else
> > +    { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
> > +#endif
> 
> 
> The only thing I'd change is preferring positive tests to negative ones.  So 
> swap the order of these lines, and the sense of the #if.

No problem. Shall I do a full resend for that, or can it be fixed up by
whoever applies?

Cheers
James

> 
> Leon, do you want to take this as a mips maintainer, or shall I as tcg maintainer?
> 
> 
> r~

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

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

[Richard Henderson]

On 10/07/2015 09:34 PM, James Hogan wrote:
>>>       { INDEX_op_brcond_i32, { "rZ", "rZ" } },
>>> +#if !use_mips32r6_instructions
>>>       { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
>>> +#else
>>> +    { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
>>> +#endif
>>
>>
>> The only thing I'd change is preferring positive tests to negative ones.  So
>> swap the order of these lines, and the sense of the #if.
>
> No problem. Shall I do a full resend for that, or can it be fixed up by
> whoever applies?

No resend needed.  I'll fix it when applying to my tcg queue.


r~

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

[Leon Alrae]

On 07/10/15 10:46, Richard Henderson wrote:
> Leon, do you want to take this as a mips maintainer, or shall I as tcg
> maintainer?

I thought this would go via Aurelien's mips tcg-backend queue. But if
Aurelien is busy, could you take them? (at the moment I don't have
anything handy to test the mips backend).

Thanks,
Leon

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

[Aurelien Jarno]

On 2015-10-07 12:47, Leon Alrae wrote:
> On 07/10/15 10:46, Richard Henderson wrote:
> > Leon, do you want to take this as a mips maintainer, or shall I as tcg
> > maintainer?
> 
> I thought this would go via Aurelien's mips tcg-backend queue. But if
> Aurelien is busy, could you take them? (at the moment I don't have
> anything handy to test the mips backend).

Sorry I have been indeed a bit busy. I can send a pull request in the
next days. As you prefer.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

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

[Aurelien Jarno]

On 2015-10-02 13:24, James Hogan wrote:
> 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>
> ---
> Changes in v3:
> - Switch to using bool eqz to indicate whether to use SELEQZ / MOVZ
>   instead of SELNEZ / MOVN (Richard).
> - Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment to remind
>   reader that it should be guaranteed via constraints (Richard).
> 
> Changes in v2:
> - Combine with patch 6 from v1, and drop functional changes to movcond
>   implementation pre-r6. We now provide different constraints for
>   movcond depending on presence of r6. (thanks Richard for feedback).
> ---
>  tcg/mips/tcg-target.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

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

[James Hogan]

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

On Thu, Oct 08, 2015 at 06:31:32PM +0200, Aurelien Jarno wrote:
> On 2015-10-02 13:24, James Hogan wrote:
> > 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>
> > ---
> > Changes in v3:
> > - Switch to using bool eqz to indicate whether to use SELEQZ / MOVZ
> >   instead of SELNEZ / MOVN (Richard).
> > - Add tcg_debug_assert(v2 == ret) for pre-r6 case with comment to remind
> >   reader that it should be guaranteed via constraints (Richard).
> > 
> > Changes in v2:
> > - Combine with patch 6 from v1, and drop functional changes to movcond
> >   implementation pre-r6. We now provide different constraints for
> >   movcond depending on presence of r6. (thanks Richard for feedback).
> > ---
> >  tcg/mips/tcg-target.c | 43 +++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

Thanks for the reviews Aurelien!

Cheers
James

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