qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/mips: Fix loongson multimedia condition instructions
@ 2020-03-21  4:56 Jiaxun Yang
  2020-03-21  9:08 ` Aleksandar Markovic
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jiaxun Yang @ 2020-03-21  4:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jiaxun Yang, aleksandar.qemu.devel, aleksandar.rikalo, aurelien

Loongson multimedia condition instructions were previously implemented as
write 0 to rd due to lack of documentation. So I just confirmed with Loongson
about their encoding and implemented them correctly.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 target/mips/translate.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index d745bd2803..43be8d27b5 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -5529,6 +5529,8 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
 {
     uint32_t opc, shift_max;
     TCGv_i64 t0, t1;
+    TCGCond cond;
+    TCGLabel *lab;
 
     opc = MASK_LMI(ctx->opcode);
     switch (opc) {
@@ -5816,7 +5818,7 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
     case OPC_DADD_CP2:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
-            TCGLabel *lab = gen_new_label();
+            lab = gen_new_label();
 
             tcg_gen_mov_i64(t2, t0);
             tcg_gen_add_i64(t0, t1, t2);
@@ -5837,7 +5839,7 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
     case OPC_DSUB_CP2:
         {
             TCGv_i64 t2 = tcg_temp_new_i64();
-            TCGLabel *lab = gen_new_label();
+            lab = gen_new_label();
 
             tcg_gen_mov_i64(t2, t0);
             tcg_gen_sub_i64(t0, t1, t2);
@@ -5862,14 +5864,39 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
 
     case OPC_SEQU_CP2:
     case OPC_SEQ_CP2:
+        cond = TCG_COND_EQ;
+        goto do_cc_cond;
+        break;
+
     case OPC_SLTU_CP2:
+        cond = TCG_COND_LTU;
+        goto do_cc_cond;
+        break;
+
     case OPC_SLT_CP2:
+        cond = TCG_COND_LT;
+        goto do_cc_cond;
+        break;
+
     case OPC_SLEU_CP2:
+        cond = TCG_COND_LEU;
+        goto do_cc_cond;
+        break;
+
     case OPC_SLE_CP2:
-        /*
-         * ??? Document is unclear: Set FCC[CC].  Does that mean the
-         * FD field is the CC field?
-         */
+        cond = TCG_COND_LE;
+    do_cc_cond:
+        {
+            int cc = (ctx->opcode >> 8) & 0x7;
+            lab = gen_new_label();
+            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
+            tcg_gen_brcond_i64(cond, t0, t1, lab);
+            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
+            gen_set_label(lab);
+        }
+        goto no_rd;
+        break;
+
     default:
         MIPS_INVAL("loongson_cp2");
         generate_exception_end(ctx, EXCP_RI);
@@ -5878,6 +5905,7 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
 
     gen_store_fpr64(ctx, t0, rd);
 
+no_rd:
     tcg_temp_free_i64(t0);
     tcg_temp_free_i64(t1);
 }
-- 
2.26.0.rc2




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

* Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
  2020-03-21  4:56 [PATCH] target/mips: Fix loongson multimedia condition instructions Jiaxun Yang
@ 2020-03-21  9:08 ` Aleksandar Markovic
  2020-03-21 10:39 ` Philippe Mathieu-Daudé
  2020-03-23 17:36 ` Richard Henderson
  2 siblings, 0 replies; 7+ messages in thread
From: Aleksandar Markovic @ 2020-03-21  9:08 UTC (permalink / raw)
  To: Jiaxun Yang; +Cc: aleksandar.rikalo, qemu-devel, aurelien

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

5:58 AM Sub, 21.03.2020. Jiaxun Yang <jiaxun.yang@flygoat.com> је
написао/ла:
>
> Loongson multimedia condition instructions were previously implemented as
> write 0 to rd due to lack of documentation. So I just confirmed with
Loongson
> about their encoding and implemented them correctly.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---

Thanks, Jiaxun!

I feel relieved that this "mistery" is about to be solved. This patch
actually deals with a long-standing known bug, and, on that basis, it is
eligible to be integrated into QEMU 5.0 after soft-freeze.

I'll take a closer look at the details and their possible improvements in
next few days. But, again, in general, I salute this patch.

Yours,
Aleksansdar

>  target/mips/translate.c | 40 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index d745bd2803..43be8d27b5 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -5529,6 +5529,8 @@ static void gen_loongson_multimedia(DisasContext
*ctx, int rd, int rs, int rt)
>  {
>      uint32_t opc, shift_max;
>      TCGv_i64 t0, t1;
> +    TCGCond cond;
> +    TCGLabel *lab;
>
>      opc = MASK_LMI(ctx->opcode);
>      switch (opc) {
> @@ -5816,7 +5818,7 @@ static void gen_loongson_multimedia(DisasContext
*ctx, int rd, int rs, int rt)
>      case OPC_DADD_CP2:
>          {
>              TCGv_i64 t2 = tcg_temp_new_i64();
> -            TCGLabel *lab = gen_new_label();
> +            lab = gen_new_label();
>
>              tcg_gen_mov_i64(t2, t0);
>              tcg_gen_add_i64(t0, t1, t2);
> @@ -5837,7 +5839,7 @@ static void gen_loongson_multimedia(DisasContext
*ctx, int rd, int rs, int rt)
>      case OPC_DSUB_CP2:
>          {
>              TCGv_i64 t2 = tcg_temp_new_i64();
> -            TCGLabel *lab = gen_new_label();
> +            lab = gen_new_label();
>
>              tcg_gen_mov_i64(t2, t0);
>              tcg_gen_sub_i64(t0, t1, t2);
> @@ -5862,14 +5864,39 @@ static void gen_loongson_multimedia(DisasContext
*ctx, int rd, int rs, int rt)
>
>      case OPC_SEQU_CP2:
>      case OPC_SEQ_CP2:
> +        cond = TCG_COND_EQ;
> +        goto do_cc_cond;
> +        break;
> +
>      case OPC_SLTU_CP2:
> +        cond = TCG_COND_LTU;
> +        goto do_cc_cond;
> +        break;
> +
>      case OPC_SLT_CP2:
> +        cond = TCG_COND_LT;
> +        goto do_cc_cond;
> +        break;
> +
>      case OPC_SLEU_CP2:
> +        cond = TCG_COND_LEU;
> +        goto do_cc_cond;
> +        break;
> +
>      case OPC_SLE_CP2:
> -        /*
> -         * ??? Document is unclear: Set FCC[CC].  Does that mean the
> -         * FD field is the CC field?
> -         */
> +        cond = TCG_COND_LE;
> +    do_cc_cond:
> +        {
> +            int cc = (ctx->opcode >> 8) & 0x7;
> +            lab = gen_new_label();
> +            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
> +            tcg_gen_brcond_i64(cond, t0, t1, lab);
> +            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
> +            gen_set_label(lab);
> +        }
> +        goto no_rd;
> +        break;
> +
>      default:
>          MIPS_INVAL("loongson_cp2");
>          generate_exception_end(ctx, EXCP_RI);
> @@ -5878,6 +5905,7 @@ static void gen_loongson_multimedia(DisasContext
*ctx, int rd, int rs, int rt)
>
>      gen_store_fpr64(ctx, t0, rd);
>
> +no_rd:
>      tcg_temp_free_i64(t0);
>      tcg_temp_free_i64(t1);
>  }
> --
> 2.26.0.rc2
>
>
>

[-- Attachment #2: Type: text/html, Size: 4890 bytes --]

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

* Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
  2020-03-21  4:56 [PATCH] target/mips: Fix loongson multimedia condition instructions Jiaxun Yang
  2020-03-21  9:08 ` Aleksandar Markovic
@ 2020-03-21 10:39 ` Philippe Mathieu-Daudé
  2020-03-21 10:57   ` Jiaxun Yang
  2020-03-23 17:36 ` Richard Henderson
  2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-21 10:39 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: aleksandar.qemu.devel, aleksandar.rikalo, aurelien

On 3/21/20 5:56 AM, Jiaxun Yang wrote:
> Loongson multimedia condition instructions were previously implemented as
> write 0 to rd due to lack of documentation. So I just confirmed with Loongson
> about their encoding and implemented them correctly.

Can you refer to the datasheet in the commit message, or have someone 
from Loongson Technology, Lemote Tech or with access to the specs ack 
your patch?

> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   target/mips/translate.c | 40 ++++++++++++++++++++++++++++++++++------
>   1 file changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index d745bd2803..43be8d27b5 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -5529,6 +5529,8 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>   {
>       uint32_t opc, shift_max;
>       TCGv_i64 t0, t1;
> +    TCGCond cond;
> +    TCGLabel *lab;
>   
>       opc = MASK_LMI(ctx->opcode);
>       switch (opc) {
> @@ -5816,7 +5818,7 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>       case OPC_DADD_CP2:
>           {
>               TCGv_i64 t2 = tcg_temp_new_i64();
> -            TCGLabel *lab = gen_new_label();
> +            lab = gen_new_label();
>   
>               tcg_gen_mov_i64(t2, t0);
>               tcg_gen_add_i64(t0, t1, t2);
> @@ -5837,7 +5839,7 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>       case OPC_DSUB_CP2:
>           {
>               TCGv_i64 t2 = tcg_temp_new_i64();
> -            TCGLabel *lab = gen_new_label();
> +            lab = gen_new_label();
>   
>               tcg_gen_mov_i64(t2, t0);
>               tcg_gen_sub_i64(t0, t1, t2);
> @@ -5862,14 +5864,39 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>   
>       case OPC_SEQU_CP2:
>       case OPC_SEQ_CP2:
> +        cond = TCG_COND_EQ;
> +        goto do_cc_cond;
> +        break;
> +
>       case OPC_SLTU_CP2:
> +        cond = TCG_COND_LTU;
> +        goto do_cc_cond;
> +        break;
> +
>       case OPC_SLT_CP2:
> +        cond = TCG_COND_LT;
> +        goto do_cc_cond;
> +        break;
> +
>       case OPC_SLEU_CP2:
> +        cond = TCG_COND_LEU;
> +        goto do_cc_cond;
> +        break;
> +
>       case OPC_SLE_CP2:
> -        /*
> -         * ??? Document is unclear: Set FCC[CC].  Does that mean the
> -         * FD field is the CC field?
> -         */
> +        cond = TCG_COND_LE;
> +    do_cc_cond:
> +        {
> +            int cc = (ctx->opcode >> 8) & 0x7;
> +            lab = gen_new_label();
> +            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
> +            tcg_gen_brcond_i64(cond, t0, t1, lab);
> +            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
> +            gen_set_label(lab);
> +        }
> +        goto no_rd;
> +        break;
> +
>       default:
>           MIPS_INVAL("loongson_cp2");
>           generate_exception_end(ctx, EXCP_RI);
> @@ -5878,6 +5905,7 @@ static void gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>   
>       gen_store_fpr64(ctx, t0, rd);
>   
> +no_rd:
>       tcg_temp_free_i64(t0);
>       tcg_temp_free_i64(t1);
>   }
> 



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

* Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
  2020-03-21 10:39 ` Philippe Mathieu-Daudé
@ 2020-03-21 10:57   ` Jiaxun Yang
  2020-03-21 11:17     ` Jiaxun Yang
  0 siblings, 1 reply; 7+ messages in thread
From: Jiaxun Yang @ 2020-03-21 10:57 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: aleksandar.qemu.devel, aleksandar.rikalo, aurelien



于 2020年3月21日 GMT+08:00 下午6:39:21, "Philippe Mathieu-Daudé" <philmd@redhat.com> 写到:
>On 3/21/20 5:56 AM, Jiaxun Yang wrote:
>> Loongson multimedia condition instructions were previously
>implemented as
>> write 0 to rd due to lack of documentation. So I just confirmed with
>Loongson
>> about their encoding and implemented them correctly.
>
>Can you refer to the datasheet in the commit message, or have someone 
>from Loongson Technology, Lemote Tech or with access to the specs ack 
>your patch?

I just confirmed with Loongson guys on IM.

+ Huacai

Hi Huacai,

Could you please acknowledge this change,
Thanks.

--
Jiaxun Yang

>
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   target/mips/translate.c | 40
>++++++++++++++++++++++++++++++++++------
>>   1 file changed, 34 insertions(+), 6 deletions(-)
>> 
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index d745bd2803..43be8d27b5 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -5529,6 +5529,8 @@ static void
>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>   {
>>       uint32_t opc, shift_max;
>>       TCGv_i64 t0, t1;
>> +    TCGCond cond;
>> +    TCGLabel *lab;
>>   
>>       opc = MASK_LMI(ctx->opcode);
>>       switch (opc) {
>> @@ -5816,7 +5818,7 @@ static void
>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>       case OPC_DADD_CP2:
>>           {
>>               TCGv_i64 t2 = tcg_temp_new_i64();
>> -            TCGLabel *lab = gen_new_label();
>> +            lab = gen_new_label();
>>   
>>               tcg_gen_mov_i64(t2, t0);
>>               tcg_gen_add_i64(t0, t1, t2);
>> @@ -5837,7 +5839,7 @@ static void
>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>       case OPC_DSUB_CP2:
>>           {
>>               TCGv_i64 t2 = tcg_temp_new_i64();
>> -            TCGLabel *lab = gen_new_label();
>> +            lab = gen_new_label();
>>   
>>               tcg_gen_mov_i64(t2, t0);
>>               tcg_gen_sub_i64(t0, t1, t2);
>> @@ -5862,14 +5864,39 @@ static void
>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>   
>>       case OPC_SEQU_CP2:
>>       case OPC_SEQ_CP2:
>> +        cond = TCG_COND_EQ;
>> +        goto do_cc_cond;
>> +        break;
>> +
>>       case OPC_SLTU_CP2:
>> +        cond = TCG_COND_LTU;
>> +        goto do_cc_cond;
>> +        break;
>> +
>>       case OPC_SLT_CP2:
>> +        cond = TCG_COND_LT;
>> +        goto do_cc_cond;
>> +        break;
>> +
>>       case OPC_SLEU_CP2:
>> +        cond = TCG_COND_LEU;
>> +        goto do_cc_cond;
>> +        break;
>> +
>>       case OPC_SLE_CP2:
>> -        /*
>> -         * ??? Document is unclear: Set FCC[CC].  Does that mean the
>> -         * FD field is the CC field?
>> -         */
>> +        cond = TCG_COND_LE;
>> +    do_cc_cond:
>> +        {
>> +            int cc = (ctx->opcode >> 8) & 0x7;
>> +            lab = gen_new_label();
>> +            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 <<
>get_fp_bit(cc));
>> +            tcg_gen_brcond_i64(cond, t0, t1, lab);
>> +            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 <<
>get_fp_bit(cc));
>> +            gen_set_label(lab);
>> +        }
>> +        goto no_rd;
>> +        break;
>> +
>>       default:
>>           MIPS_INVAL("loongson_cp2");
>>           generate_exception_end(ctx, EXCP_RI);
>> @@ -5878,6 +5905,7 @@ static void
>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>   
>>       gen_store_fpr64(ctx, t0, rd);
>>   
>> +no_rd:
>>       tcg_temp_free_i64(t0);
>>       tcg_temp_free_i64(t1);
>>   }
>> 

-- 
Jiaxun Yang


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

* Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
  2020-03-21 10:57   ` Jiaxun Yang
@ 2020-03-21 11:17     ` Jiaxun Yang
  2020-03-21 11:30       ` Huacai Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Jiaxun Yang @ 2020-03-21 11:17 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: chenhuacai, chenhc, aleksandar.qemu.devel, aleksandar.rikalo, aurelien



于 2020年3月21日 GMT+08:00 下午6:57:54, Jiaxun Yang <jiaxun.yang@flygoat.com> 写到:
>
>
>于 2020年3月21日 GMT+08:00 下午6:39:21, "Philippe Mathieu-Daudé"
><philmd@redhat.com> 写到:
>>On 3/21/20 5:56 AM, Jiaxun Yang wrote:
>>> Loongson multimedia condition instructions were previously
>>implemented as
>>> write 0 to rd due to lack of documentation. So I just confirmed with
>>Loongson
>>> about their encoding and implemented them correctly.
>>
>>Can you refer to the datasheet in the commit message, or have someone 
>>from Loongson Technology, Lemote Tech or with access to the specs ack 
>>your patch?
>
>I just confirmed with Loongson guys on IM.
>
>+ Huacai

+Huacai's Gmail as his Lemote mail rejected my bounce.

>
>Hi Huacai,
>
>Could you please acknowledge this change,
>Thanks.
>
>--
>Jiaxun Yang
>
>>
>>> 
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>   target/mips/translate.c | 40
>>++++++++++++++++++++++++++++++++++------
>>>   1 file changed, 34 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>>> index d745bd2803..43be8d27b5 100644
>>> --- a/target/mips/translate.c
>>> +++ b/target/mips/translate.c
>>> @@ -5529,6 +5529,8 @@ static void
>>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>>   {
>>>       uint32_t opc, shift_max;
>>>       TCGv_i64 t0, t1;
>>> +    TCGCond cond;
>>> +    TCGLabel *lab;
>>>   
>>>       opc = MASK_LMI(ctx->opcode);
>>>       switch (opc) {
>>> @@ -5816,7 +5818,7 @@ static void
>>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>>       case OPC_DADD_CP2:
>>>           {
>>>               TCGv_i64 t2 = tcg_temp_new_i64();
>>> -            TCGLabel *lab = gen_new_label();
>>> +            lab = gen_new_label();
>>>   
>>>               tcg_gen_mov_i64(t2, t0);
>>>               tcg_gen_add_i64(t0, t1, t2);
>>> @@ -5837,7 +5839,7 @@ static void
>>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>>       case OPC_DSUB_CP2:
>>>           {
>>>               TCGv_i64 t2 = tcg_temp_new_i64();
>>> -            TCGLabel *lab = gen_new_label();
>>> +            lab = gen_new_label();
>>>   
>>>               tcg_gen_mov_i64(t2, t0);
>>>               tcg_gen_sub_i64(t0, t1, t2);
>>> @@ -5862,14 +5864,39 @@ static void
>>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>>   
>>>       case OPC_SEQU_CP2:
>>>       case OPC_SEQ_CP2:
>>> +        cond = TCG_COND_EQ;
>>> +        goto do_cc_cond;
>>> +        break;
>>> +
>>>       case OPC_SLTU_CP2:
>>> +        cond = TCG_COND_LTU;
>>> +        goto do_cc_cond;
>>> +        break;
>>> +
>>>       case OPC_SLT_CP2:
>>> +        cond = TCG_COND_LT;
>>> +        goto do_cc_cond;
>>> +        break;
>>> +
>>>       case OPC_SLEU_CP2:
>>> +        cond = TCG_COND_LEU;
>>> +        goto do_cc_cond;
>>> +        break;
>>> +
>>>       case OPC_SLE_CP2:
>>> -        /*
>>> -         * ??? Document is unclear: Set FCC[CC].  Does that mean
>the
>>> -         * FD field is the CC field?
>>> -         */
>>> +        cond = TCG_COND_LE;
>>> +    do_cc_cond:
>>> +        {
>>> +            int cc = (ctx->opcode >> 8) & 0x7;
>>> +            lab = gen_new_label();
>>> +            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 <<
>>get_fp_bit(cc));
>>> +            tcg_gen_brcond_i64(cond, t0, t1, lab);
>>> +            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 <<
>>get_fp_bit(cc));
>>> +            gen_set_label(lab);
>>> +        }
>>> +        goto no_rd;
>>> +        break;
>>> +
>>>       default:
>>>           MIPS_INVAL("loongson_cp2");
>>>           generate_exception_end(ctx, EXCP_RI);
>>> @@ -5878,6 +5905,7 @@ static void
>>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
>>>   
>>>       gen_store_fpr64(ctx, t0, rd);
>>>   
>>> +no_rd:
>>>       tcg_temp_free_i64(t0);
>>>       tcg_temp_free_i64(t1);
>>>   }
>>> 

-- 
Jiaxun Yang


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

* Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
  2020-03-21 11:17     ` Jiaxun Yang
@ 2020-03-21 11:30       ` Huacai Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Huacai Chen @ 2020-03-21 11:30 UTC (permalink / raw)
  To: Jiaxun Yang
  Cc: aleksandar.qemu.devel, aleksandar.rikalo,
	Philippe Mathieu-Daudé,
	qemu-devel, Aurelien Jarno

On Sat, Mar 21, 2020 at 7:17 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 于 2020年3月21日 GMT+08:00 下午6:57:54, Jiaxun Yang <jiaxun.yang@flygoat.com> 写到:
> >
> >
> >于 2020年3月21日 GMT+08:00 下午6:39:21, "Philippe Mathieu-Daudé"
> ><philmd@redhat.com> 写到:
> >>On 3/21/20 5:56 AM, Jiaxun Yang wrote:
> >>> Loongson multimedia condition instructions were previously
> >>implemented as
> >>> write 0 to rd due to lack of documentation. So I just confirmed with
> >>Loongson
> >>> about their encoding and implemented them correctly.
> >>
> >>Can you refer to the datasheet in the commit message, or have someone
> >>from Loongson Technology, Lemote Tech or with access to the specs ack
> >>your patch?
> >
> >I just confirmed with Loongson guys on IM.
> >
> >+ Huacai
>
> +Huacai's Gmail as his Lemote mail rejected my bounce.
>
> >
> >Hi Huacai,
> >
> >Could you please acknowledge this change,
> >Thanks.
> >
> >--
> >Jiaxun Yang
Acked-by: Huacai Chen <chenhc@lemote.com>

> >
> >>
> >>>
> >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >>> ---
> >>>   target/mips/translate.c | 40
> >>++++++++++++++++++++++++++++++++++------
> >>>   1 file changed, 34 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/target/mips/translate.c b/target/mips/translate.c
> >>> index d745bd2803..43be8d27b5 100644
> >>> --- a/target/mips/translate.c
> >>> +++ b/target/mips/translate.c
> >>> @@ -5529,6 +5529,8 @@ static void
> >>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
> >>>   {
> >>>       uint32_t opc, shift_max;
> >>>       TCGv_i64 t0, t1;
> >>> +    TCGCond cond;
> >>> +    TCGLabel *lab;
> >>>
> >>>       opc = MASK_LMI(ctx->opcode);
> >>>       switch (opc) {
> >>> @@ -5816,7 +5818,7 @@ static void
> >>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
> >>>       case OPC_DADD_CP2:
> >>>           {
> >>>               TCGv_i64 t2 = tcg_temp_new_i64();
> >>> -            TCGLabel *lab = gen_new_label();
> >>> +            lab = gen_new_label();
> >>>
> >>>               tcg_gen_mov_i64(t2, t0);
> >>>               tcg_gen_add_i64(t0, t1, t2);
> >>> @@ -5837,7 +5839,7 @@ static void
> >>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
> >>>       case OPC_DSUB_CP2:
> >>>           {
> >>>               TCGv_i64 t2 = tcg_temp_new_i64();
> >>> -            TCGLabel *lab = gen_new_label();
> >>> +            lab = gen_new_label();
> >>>
> >>>               tcg_gen_mov_i64(t2, t0);
> >>>               tcg_gen_sub_i64(t0, t1, t2);
> >>> @@ -5862,14 +5864,39 @@ static void
> >>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
> >>>
> >>>       case OPC_SEQU_CP2:
> >>>       case OPC_SEQ_CP2:
> >>> +        cond = TCG_COND_EQ;
> >>> +        goto do_cc_cond;
> >>> +        break;
> >>> +
> >>>       case OPC_SLTU_CP2:
> >>> +        cond = TCG_COND_LTU;
> >>> +        goto do_cc_cond;
> >>> +        break;
> >>> +
> >>>       case OPC_SLT_CP2:
> >>> +        cond = TCG_COND_LT;
> >>> +        goto do_cc_cond;
> >>> +        break;
> >>> +
> >>>       case OPC_SLEU_CP2:
> >>> +        cond = TCG_COND_LEU;
> >>> +        goto do_cc_cond;
> >>> +        break;
> >>> +
> >>>       case OPC_SLE_CP2:
> >>> -        /*
> >>> -         * ??? Document is unclear: Set FCC[CC].  Does that mean
> >the
> >>> -         * FD field is the CC field?
> >>> -         */
> >>> +        cond = TCG_COND_LE;
> >>> +    do_cc_cond:
> >>> +        {
> >>> +            int cc = (ctx->opcode >> 8) & 0x7;
> >>> +            lab = gen_new_label();
> >>> +            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 <<
> >>get_fp_bit(cc));
> >>> +            tcg_gen_brcond_i64(cond, t0, t1, lab);
> >>> +            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 <<
> >>get_fp_bit(cc));
> >>> +            gen_set_label(lab);
> >>> +        }
> >>> +        goto no_rd;
> >>> +        break;
> >>> +
> >>>       default:
> >>>           MIPS_INVAL("loongson_cp2");
> >>>           generate_exception_end(ctx, EXCP_RI);
> >>> @@ -5878,6 +5905,7 @@ static void
> >>gen_loongson_multimedia(DisasContext *ctx, int rd, int rs, int rt)
> >>>
> >>>       gen_store_fpr64(ctx, t0, rd);
> >>>
> >>> +no_rd:
> >>>       tcg_temp_free_i64(t0);
> >>>       tcg_temp_free_i64(t1);
> >>>   }
> >>>
>
> --
> Jiaxun Yang


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

* Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
  2020-03-21  4:56 [PATCH] target/mips: Fix loongson multimedia condition instructions Jiaxun Yang
  2020-03-21  9:08 ` Aleksandar Markovic
  2020-03-21 10:39 ` Philippe Mathieu-Daudé
@ 2020-03-23 17:36 ` Richard Henderson
  2 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2020-03-23 17:36 UTC (permalink / raw)
  To: Jiaxun Yang, qemu-devel
  Cc: aleksandar.qemu.devel, aleksandar.rikalo, aurelien

On 3/20/20 9:56 PM, Jiaxun Yang wrote:
>      case OPC_SLE_CP2:
> -        /*
> -         * ??? Document is unclear: Set FCC[CC].  Does that mean the
> -         * FD field is the CC field?
> -         */
> +        cond = TCG_COND_LE;
> +    do_cc_cond:
> +        {
> +            int cc = (ctx->opcode >> 8) & 0x7;
> +            lab = gen_new_label();
> +            tcg_gen_ori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
> +            tcg_gen_brcond_i64(cond, t0, t1, lab);
> +            tcg_gen_xori_i32(fpu_fcr31, fpu_fcr31, 1 << get_fp_bit(cc));
> +            gen_set_label(lab);
> +        }
> +        goto no_rd;
> +        break;

There is no need for a branch here.  This is a deposit operation.

    TCGv_i64 t64 = tcg_temp_new_i64();
    TCGv_i32 t32 = tcg_temp_new_i32();

    tcg_gen_setcond_i64(cond, t64, t0, t1);
    tcg_gen_extrl_i64_i32(t32, t64);
    tcg_gen_deposit_i32(cpu_fcr31, cpu_fcr31, t32,
                        get_fp_bit(cc), 1);

    tcg_temp_free_i32(t32);
    tcg_temp_free_i64(t64);


r~


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

end of thread, other threads:[~2020-03-23 17:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-21  4:56 [PATCH] target/mips: Fix loongson multimedia condition instructions Jiaxun Yang
2020-03-21  9:08 ` Aleksandar Markovic
2020-03-21 10:39 ` Philippe Mathieu-Daudé
2020-03-21 10:57   ` Jiaxun Yang
2020-03-21 11:17     ` Jiaxun Yang
2020-03-21 11:30       ` Huacai Chen
2020-03-23 17:36 ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).