1. Patchew
  2. QEMU
  3. View series

[PATCH] target/mips: Fix loongson multimedia condition instructions

Jiaxun Yang posted 1 patch 4 years, 1 month ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200321045621.2139953-1-jiaxun.yang@flygoat.com
Maintainers: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>
There is a newer version of this series
target/mips/translate.c | 40 ++++++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
[PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Jiaxun Yang 4 years, 1 month ago 
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
Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Aleksandar Markovic 4 years, 1 month ago 
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
>
>
>
Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Philippe Mathieu-Daudé 4 years, 1 month ago 
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);
>   }
>
Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Jiaxun Yang 4 years, 1 month ago 

于 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
Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Jiaxun Yang 4 years, 1 month ago 

于 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
Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Huacai Chen 4 years, 1 month ago 
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
Re: [PATCH] target/mips: Fix loongson multimedia condition instructions
Posted by Richard Henderson 4 years, 1 month ago 
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~

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.