[PATCH] tcg: Fixes set const_args[i] wrong value when instructions imm is 0

[PATCH] tcg: Fixes set const_args[i] wrong value when instructions imm is 0

[Song Gao]

It seems that tcg_reg_alloc_op() set const_args[i] wrong value
when instructions imm is 0. The LoongArch tcg_out_vec_op() cmp_vec
use the wrong const_args[2].
e.g
   The wrong const_args[2] is 0.
   IN: vslti.w v5, v4, 0x0   OUT: vslt.w  v1, v1, v0

   The right const_args[2] is 1.
   IN: vslti.w v5, v4, 0x0   OUT: vslti.w v1, v1, 0x0

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2136
Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 tcg/tcg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index e2c38f6d11..5b290123bc 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4808,7 +4808,7 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op)
         arg_ct = &def->args_ct[i];
         ts = arg_temp(arg);
 
-        if (ts->val_type == TEMP_VAL_CONST
+        if ((ts->val_type == TEMP_VAL_CONST || ts->kind == TEMP_CONST)
             && tcg_target_const_match(ts->val, ts->type, arg_ct->ct, TCGOP_VECE(op))) {
             /* constant is OK for instruction */
             const_args[i] = 1;
-- 
2.25.1

Re: [PATCH] tcg: Fixes set const_args[i] wrong value when instructions imm is 0

[Richard Henderson]

On 1/31/24 17:27, Song Gao wrote:
> It seems that tcg_reg_alloc_op() set const_args[i] wrong value
> when instructions imm is 0. The LoongArch tcg_out_vec_op() cmp_vec
> use the wrong const_args[2].
> e.g
>     The wrong const_args[2] is 0.
>     IN: vslti.w v5, v4, 0x0   OUT: vslt.w  v1, v1, v0
> 
>     The right const_args[2] is 1.
>     IN: vslti.w v5, v4, 0x0   OUT: vslti.w v1, v1, 0x0
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2136
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   tcg/tcg.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index e2c38f6d11..5b290123bc 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -4808,7 +4808,7 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op)
>           arg_ct = &def->args_ct[i];
>           ts = arg_temp(arg);
>   
> -        if (ts->val_type == TEMP_VAL_CONST
> +        if ((ts->val_type == TEMP_VAL_CONST || ts->kind == TEMP_CONST)
>               && tcg_target_const_match(ts->val, ts->type, arg_ct->ct, TCGOP_VECE(op))) {
>               /* constant is OK for instruction */
>               const_args[i] = 1;

This is wrong.

I strongly suspect that the TEMP_CONST value 0 has been loaded into a register for use in 
another operation, and the register allocator sees that it is still there.


r~

Re: [PATCH] tcg: Fixes set const_args[i] wrong value when instructions imm is 0

[gaosong]

在 2024/2/1 上午5:16, Richard Henderson 写道:
> On 1/31/24 17:27, Song Gao wrote:
>> It seems that tcg_reg_alloc_op() set const_args[i] wrong value
>> when instructions imm is 0. The LoongArch tcg_out_vec_op() cmp_vec
>> use the wrong const_args[2].
>> e.g
>>     The wrong const_args[2] is 0.
>>     IN: vslti.w v5, v4, 0x0   OUT: vslt.w  v1, v1, v0
>>
>>     The right const_args[2] is 1.
>>     IN: vslti.w v5, v4, 0x0   OUT: vslti.w v1, v1, 0x0
>>
>> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2136
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   tcg/tcg.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tcg/tcg.c b/tcg/tcg.c
>> index e2c38f6d11..5b290123bc 100644
>> --- a/tcg/tcg.c
>> +++ b/tcg/tcg.c
>> @@ -4808,7 +4808,7 @@ static void tcg_reg_alloc_op(TCGContext *s, 
>> const TCGOp *op)
>>           arg_ct = &def->args_ct[i];
>>           ts = arg_temp(arg);
>>   -        if (ts->val_type == TEMP_VAL_CONST
>> +        if ((ts->val_type == TEMP_VAL_CONST || ts->kind == TEMP_CONST)
>>               && tcg_target_const_match(ts->val, ts->type, 
>> arg_ct->ct, TCGOP_VECE(op))) {
>>               /* constant is OK for instruction */
>>               const_args[i] = 1;
>
> This is wrong.
>
> I strongly suspect that the TEMP_CONST value 0 has been loaded into a 
> register for use in another operation, and the register allocator sees 
> that it is still there.
>
Ah, I'm not familiar with this piece of code,  I just try to fix the bug,
and thanks for your suggestion.

Thanks.
Song Gao
>
> r~