[RFC PATCH 11/42] target/mips/tx79: Move PCPYH opcode to decodetree

Philippe Mathieu-Daudé posted 42 patches 4 years, 9 months ago
There is a newer version of this series
[RFC PATCH 11/42] target/mips/tx79: Move PCPYH opcode to decodetree
Posted by Philippe Mathieu-Daudé 4 years, 9 months ago
Move the existing PCPYH opcode (Parallel Copy Halfword) to decodetree.
Remove unnecessary code / comments.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tx79.decode      |  5 +++++
 target/mips/translate.c      | 39 ------------------------------------
 target/mips/tx79_translate.c | 22 ++++++++++++++++++++
 3 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/target/mips/tx79.decode b/target/mips/tx79.decode
index 30737da54e4..7af35458b0a 100644
--- a/target/mips/tx79.decode
+++ b/target/mips/tx79.decode
@@ -17,6 +17,7 @@
 # Named instruction formats.  These are generally used to
 # reduce the amount of duplication between instruction patterns.
 
+@rt_rd          ...... ..... rt:5  rd:5  ..... ......   &rtype rs=0 sa=0
 @rs             ...... rs:5  ..... ..........  ......   &rtype rt=0 rd=0 sa=0
 @rd             ...... ..........  rd:5  ..... ......   &rtype rs=0 rt=0 sa=0
 
@@ -26,3 +27,7 @@ MFHI1           011100 0000000000  ..... 00000 010000   @rd
 MTHI1           011100 .....  0000000000 00000 010001   @rs
 MFLO1           011100 0000000000  ..... 00000 010010   @rd
 MTLO1           011100 .....  0000000000 00000 010011   @rs
+
+# MMI3
+
+PCPYH           011100 00000 ..... ..... 11011 101001   @rt_rd
diff --git a/target/mips/translate.c b/target/mips/translate.c
index a13ad4959b4..b81a66ed373 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24733,42 +24733,6 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
  *                     PEXTUW
  */
 
-/*
- *  PCPYH rd, rt
- *
- *    Parallel Copy Halfword
- *
- *   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
- *  +-----------+---------+---------+---------+---------+-----------+
- *  |    MMI    |0 0 0 0 0|   rt    |   rd    |  PCPYH  |    MMI3   |
- *  +-----------+---------+---------+---------+---------+-----------+
- */
-static void gen_mmi_pcpyh(DisasContext *ctx)
-{
-    uint32_t pd, rt, rd;
-    uint32_t opcode;
-
-    opcode = ctx->opcode;
-
-    pd = extract32(opcode, 21, 5);
-    rt = extract32(opcode, 16, 5);
-    rd = extract32(opcode, 11, 5);
-
-    if (unlikely(pd != 0)) {
-        gen_reserved_instruction(ctx);
-    } else if (rd == 0) {
-        /* nop */
-    } else if (rt == 0) {
-        tcg_gen_movi_i64(cpu_gpr[rd], 0);
-        tcg_gen_movi_i64(cpu_gpr_hi[rd], 0);
-    } else {
-        tcg_gen_deposit_i64(cpu_gpr[rd], cpu_gpr[rt], cpu_gpr[rt], 16, 16);
-        tcg_gen_deposit_i64(cpu_gpr[rd], cpu_gpr[rd], cpu_gpr[rd], 32, 32);
-        tcg_gen_deposit_i64(cpu_gpr_hi[rd], cpu_gpr_hi[rt], cpu_gpr_hi[rt], 16, 16);
-        tcg_gen_deposit_i64(cpu_gpr_hi[rd], cpu_gpr_hi[rd], cpu_gpr_hi[rd], 32, 32);
-    }
-}
-
 /*
  *  PCPYLD rd, rs, rt
  *
@@ -27923,9 +27887,6 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
     case MMI_OPC_3_PEXCW:      /* TODO: MMI_OPC_3_PEXCW */
         gen_reserved_instruction(ctx); /* TODO: MMI_OPC_CLASS_MMI3 */
         break;
-    case MMI_OPC_3_PCPYH:
-        gen_mmi_pcpyh(ctx);
-        break;
     case MMI_OPC_3_PCPYUD:
         gen_mmi_pcpyud(ctx);
         break;
diff --git a/target/mips/tx79_translate.c b/target/mips/tx79_translate.c
index 905245cece7..d58b4fcd7b3 100644
--- a/target/mips/tx79_translate.c
+++ b/target/mips/tx79_translate.c
@@ -49,3 +49,25 @@ static bool trans_MTLO1(DisasContext *ctx, arg_rtype *a)
 
     return true;
 }
+
+/* Parallel Copy Halfword */
+static bool trans_PCPYH(DisasContext *s, arg_rtype *a)
+{
+    if (a->rd == 0) {
+        /* nop */
+        return true;
+    }
+
+    if (a->rt == 0) {
+        tcg_gen_movi_i64(cpu_gpr[a->rd], 0);
+        tcg_gen_movi_i64(cpu_gpr_hi[a->rd], 0);
+        return true;
+    }
+
+    tcg_gen_deposit_i64(cpu_gpr[a->rd], cpu_gpr[a->rt], cpu_gpr[a->rt], 16, 16);
+    tcg_gen_deposit_i64(cpu_gpr[a->rd], cpu_gpr[a->rd], cpu_gpr[a->rd], 32, 32);
+    tcg_gen_deposit_i64(cpu_gpr_hi[a->rd], cpu_gpr_hi[a->rt], cpu_gpr_hi[a->rt], 16, 16);
+    tcg_gen_deposit_i64(cpu_gpr_hi[a->rd], cpu_gpr_hi[a->rd], cpu_gpr_hi[a->rd], 32, 32);
+
+    return true;
+}
-- 
2.26.2

Re: [RFC PATCH 11/42] target/mips/tx79: Move PCPYH opcode to decodetree
Posted by Richard Henderson 4 years, 9 months ago
On 2/14/21 9:58 AM, Philippe Mathieu-Daudé wrote:
> +    if (a->rt == 0) {
> +        tcg_gen_movi_i64(cpu_gpr[a->rd], 0);
> +        tcg_gen_movi_i64(cpu_gpr_hi[a->rd], 0);
> +        return true;
> +    }

Is there a good reason not to use gen_load_gpr?

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 11/42] target/mips/tx79: Move PCPYH opcode to decodetree
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 2/15/21 5:26 PM, Richard Henderson wrote:
> On 2/14/21 9:58 AM, Philippe Mathieu-Daudé wrote:
>> +    if (a->rt == 0) {
>> +        tcg_gen_movi_i64(cpu_gpr[a->rd], 0);
>> +        tcg_gen_movi_i64(cpu_gpr_hi[a->rd], 0);
>> +        return true;
>> +    }
> 
> Is there a good reason not to use gen_load_gpr?

I suppose you meant gen_store_gpr*().

We need to check $rt anyway to not do the deposit calls
if it is non-zero.

As it is mostly code movement, I prefer to keep it as it
for now, we might improve it later.

> 
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

Re: [RFC PATCH 11/42] target/mips/tx79: Move PCPYH opcode to decodetree
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 3/8/21 11:48 AM, Philippe Mathieu-Daudé wrote:
> On 2/15/21 5:26 PM, Richard Henderson wrote:
>> On 2/14/21 9:58 AM, Philippe Mathieu-Daudé wrote:
>>> +    if (a->rt == 0) {
>>> +        tcg_gen_movi_i64(cpu_gpr[a->rd], 0);
>>> +        tcg_gen_movi_i64(cpu_gpr_hi[a->rd], 0);
>>> +        return true;
>>> +    }
>>
>> Is there a good reason not to use gen_load_gpr?
> 
> I suppose you meant gen_store_gpr*().

Double checking, we check for $rt, not $rd, right?

> 
> We need to check $rt anyway to not do the deposit calls
> if it is non-zero.
> 
> As it is mostly code movement, I prefer to keep it as it
> for now, we might improve it later.
> 
>>
>> Otherwise,
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Thanks!
> 

Re: [RFC PATCH 11/42] target/mips/tx79: Move PCPYH opcode to decodetree
Posted by Richard Henderson 4 years, 8 months ago
On 3/8/21 3:57 AM, Philippe Mathieu-Daudé wrote:
> On 3/8/21 11:48 AM, Philippe Mathieu-Daudé wrote:
>> On 2/15/21 5:26 PM, Richard Henderson wrote:
>>> On 2/14/21 9:58 AM, Philippe Mathieu-Daudé wrote:
>>>> +    if (a->rt == 0) {
>>>> +        tcg_gen_movi_i64(cpu_gpr[a->rd], 0);
>>>> +        tcg_gen_movi_i64(cpu_gpr_hi[a->rd], 0);
>>>> +        return true;
>>>> +    }
>>>
>>> Is there a good reason not to use gen_load_gpr?
>>
>> I suppose you meant gen_store_gpr*().
> 
> Double checking, we check for $rt, not $rd, right?

I think I meant gen_load_gpr on rt.


r~