[PATCH v2] target/loongarch: fix vldi/xvldi raise wrong error

Song Gao posted 1 patch 5 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250603024847.350568-1-gaosong@loongson.cn
Maintainers: Song Gao <gaosong@loongson.cn>
There is a newer version of this series
target/loongarch/tcg/insn_trans/trans_vec.c.inc | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
[PATCH v2] target/loongarch: fix vldi/xvldi raise wrong error
Posted by Song Gao 5 months, 2 weeks ago
on qemu we got an aborted error
**
ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
Bail out! ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
Aborted (core dumped)
bu on 3A600/3A5000 we got a "Illegal instruction" error.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971

Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 target/loongarch/tcg/insn_trans/trans_vec.c.inc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
index dff92772ad..cadc00e75e 100644
--- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
+++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
@@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, gen_helper_vmsknz_b)
 static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
 {
     int mode;
-    uint64_t data, t;
+    uint64_t data = 0, t;
 
     /*
      * imm bit [11:8] is mode, mode value is 0-12.
@@ -3570,8 +3570,7 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
         }
         break;
     default:
-        generate_exception(ctx, EXCCODE_INE);
-        g_assert_not_reached();
+        data = -1;
     }
     return data;
 }
@@ -3579,7 +3578,12 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
 static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
 {
     int sel, vece;
-    uint64_t value;
+    uint64_t value = vldi_get_value(ctx, a->imm);
+
+    if (value == -1){
+        generate_exception(ctx, EXCCODE_INE);
+        return true;
+    }
 
     if (!check_vec(ctx, oprsz)) {
         return true;
@@ -3588,7 +3592,6 @@ static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
     sel = (a->imm >> 12) & 0x1;
 
     if (sel) {
-        value = vldi_get_value(ctx, a->imm);
         vece = MO_64;
     } else {
         value = ((int32_t)(a->imm << 22)) >> 22;
-- 
2.34.1
Re: [PATCH v2] target/loongarch: fix vldi/xvldi raise wrong error
Posted by Richard Henderson 5 months, 2 weeks ago
On 6/3/25 03:48, Song Gao wrote:
> on qemu we got an aborted error
> **
> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Bail out! ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Aborted (core dumped)
> bu on 3A600/3A5000 we got a "Illegal instruction" error.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/tcg/insn_trans/trans_vec.c.inc | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> index dff92772ad..cadc00e75e 100644
> --- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> +++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> @@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, gen_helper_vmsknz_b)
>   static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>   {
>       int mode;
> -    uint64_t data, t;
> +    uint64_t data = 0, t;
>   
>       /*
>        * imm bit [11:8] is mode, mode value is 0-12.
> @@ -3570,8 +3570,7 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>           }
>           break;
>       default:
> -        generate_exception(ctx, EXCCODE_INE);
> -        g_assert_not_reached();
> +        data = -1;
>       }
>       return data;
>   }
> @@ -3579,7 +3578,12 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>   static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
>   {
>       int sel, vece;
> -    uint64_t value;
> +    uint64_t value = vldi_get_value(ctx, a->imm);
> +
> +    if (value == -1){
> +        generate_exception(ctx, EXCCODE_INE);
> +        return true;
> +    }

Incorrect.  -1 is a valid return value for several modes.


r~
Re: [PATCH v2] target/loongarch: fix vldi/xvldi raise wrong error
Posted by gaosong 5 months, 2 weeks ago
在 2025/6/3 下午3:42, Richard Henderson 写道:
> On 6/3/25 03:48, Song Gao wrote:
>> on qemu we got an aborted error
>> **
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Bail out! 
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Aborted (core dumped)
>> bu on 3A600/3A5000 we got a "Illegal instruction" error.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   target/loongarch/tcg/insn_trans/trans_vec.c.inc | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc 
>> b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> index dff92772ad..cadc00e75e 100644
>> --- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> +++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> @@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, 
>> gen_helper_vmsknz_b)
>>   static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>>   {
>>       int mode;
>> -    uint64_t data, t;
>> +    uint64_t data = 0, t;
>>         /*
>>        * imm bit [11:8] is mode, mode value is 0-12.
>> @@ -3570,8 +3570,7 @@ static uint64_t vldi_get_value(DisasContext 
>> *ctx, uint32_t imm)
>>           }
>>           break;
>>       default:
>> -        generate_exception(ctx, EXCCODE_INE);
>> -        g_assert_not_reached();
>> +        data = -1;
>>       }
>>       return data;
>>   }
>> @@ -3579,7 +3578,12 @@ static uint64_t vldi_get_value(DisasContext 
>> *ctx, uint32_t imm)
>>   static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
>>   {
>>       int sel, vece;
>> -    uint64_t value;
>> +    uint64_t value = vldi_get_value(ctx, a->imm);
>> +
>> +    if (value == -1){
>> +        generate_exception(ctx, EXCCODE_INE);
>> +        return true;
>> +    }
>
> Incorrect.  -1 is a valid return value for several modes.
>

My negligence.

thanks.
Song Gao

>
> r~