[PATCH 01/12] tcg: Fix iteration step in 32-bit gvec operation

Richard Henderson posted 12 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH 01/12] tcg: Fix iteration step in 32-bit gvec operation
Posted by Richard Henderson 2 months, 2 weeks ago
From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>

The loop in the 32-bit case of the vector compare operation
was incorrectly incrementing by 8 bytes per iteration instead
of 4 bytes. This caused the function to process only half of
the intended elements.

Cc: qemu-stable@nongnu.org
Fixes: 9622c697d1 (tcg: Add gvec compare with immediate and scalar operand)
Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20240904142739.854-2-zhiwei_liu@linux.alibaba.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tcg-op-gvec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 0308732d9b..78ee1ced80 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -3939,7 +3939,7 @@ void tcg_gen_gvec_cmps(TCGCond cond, unsigned vece, uint32_t dofs,
         uint32_t i;
 
         tcg_gen_extrl_i64_i32(t1, c);
-        for (i = 0; i < oprsz; i += 8) {
+        for (i = 0; i < oprsz; i += 4) {
             tcg_gen_ld_i32(t0, tcg_env, aofs + i);
             tcg_gen_negsetcond_i32(cond, t0, t0, t1);
             tcg_gen_st_i32(t0, tcg_env, dofs + i);
-- 
2.43.0
Re: [PATCH 01/12] tcg: Fix iteration step in 32-bit gvec operation
Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago
On 8/9/24 04:26, Richard Henderson wrote:
> From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> 
> The loop in the 32-bit case of the vector compare operation
> was incorrectly incrementing by 8 bytes per iteration instead
> of 4 bytes. This caused the function to process only half of
> the intended elements.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 9622c697d1 (tcg: Add gvec compare with immediate and scalar operand)
> Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-ID: <20240904142739.854-2-zhiwei_liu@linux.alibaba.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/tcg-op-gvec.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
> index 0308732d9b..78ee1ced80 100644
> --- a/tcg/tcg-op-gvec.c
> +++ b/tcg/tcg-op-gvec.c
> @@ -3939,7 +3939,7 @@ void tcg_gen_gvec_cmps(TCGCond cond, unsigned vece, uint32_t dofs,
>           uint32_t i;
>   
>           tcg_gen_extrl_i64_i32(t1, c);
> -        for (i = 0; i < oprsz; i += 8) {
> +        for (i = 0; i < oprsz; i += 4) {

Maybe using memop_size(vece) would have been safer to avoid copy/paste bugs?

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>               tcg_gen_ld_i32(t0, tcg_env, aofs + i);
>               tcg_gen_negsetcond_i32(cond, t0, t0, t1);
>               tcg_gen_st_i32(t0, tcg_env, dofs + i);


Re: [PATCH 01/12] tcg: Fix iteration step in 32-bit gvec operation
Posted by Richard Henderson 2 months, 2 weeks ago
On 9/9/24 23:57, Philippe Mathieu-Daudé wrote:
> On 8/9/24 04:26, Richard Henderson wrote:
>> From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
>>
>> The loop in the 32-bit case of the vector compare operation
>> was incorrectly incrementing by 8 bytes per iteration instead
>> of 4 bytes. This caused the function to process only half of
>> the intended elements.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 9622c697d1 (tcg: Add gvec compare with immediate and scalar operand)
>> Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
>> Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-ID: <20240904142739.854-2-zhiwei_liu@linux.alibaba.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   tcg/tcg-op-gvec.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
>> index 0308732d9b..78ee1ced80 100644
>> --- a/tcg/tcg-op-gvec.c
>> +++ b/tcg/tcg-op-gvec.c
>> @@ -3939,7 +3939,7 @@ void tcg_gen_gvec_cmps(TCGCond cond, unsigned vece, uint32_t dofs,
>>           uint32_t i;
>>           tcg_gen_extrl_i64_i32(t1, c);
>> -        for (i = 0; i < oprsz; i += 8) {
>> +        for (i = 0; i < oprsz; i += 4) {
> 
> Maybe using memop_size(vece) would have been safer to avoid copy/paste bugs?

No, vece can be smaller than 4 here.

r~

> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>>               tcg_gen_ld_i32(t0, tcg_env, aofs + i);
>>               tcg_gen_negsetcond_i32(cond, t0, t0, t1);
>>               tcg_gen_st_i32(t0, tcg_env, dofs + i);
>