Re: [PATCH] target/arm: Fix SVE2 WHILEWR/WHILERW zero diff boundary case

On 8/6/26 00:47, [email protected] wrote:
> From: YanjunYang <[email protected]>
> 
> The trans_WHILE_ptr function incorrectly handles the case where the
> address difference divided by ESIZE results in zero. This happens when
> the address difference is less than ESIZE but greater than zero.
> 
> In this boundary case, the address difference is less than ESIZE, which
> means all elements are safe from conflicts. The previous code would set
> the predicate to all zeros, but the correct behavior is to set all
> elements to one.
> 
> Fix by checking if diff == 0 after the shift and setting diff to tmax
> (all elements true) in this case.
> 
> Fixes the following test cases:
>    whilewr p0.s, x9, x10  ; x9=8, x10=11 => p0=0xffffffff (all ones)
>    whilerw p0.s, x9, x10  ; x9=8, x10=11 => p0=0xffffffff (all ones)
> 
> Signed-off-by: YanjunYang <[email protected]>
> ---
>   target/arm/tcg/translate-sve.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
> index fc4cc8c479..33613ebf58 100644
> --- a/target/arm/tcg/translate-sve.c
> +++ b/target/arm/tcg/translate-sve.c
> @@ -3736,6 +3736,10 @@ static bool trans_WHILE_ptr(DisasContext *s, arg_WHILE_ptr *a)
>           tcg_gen_movcond_i64(TCG_COND_GEU, diff, op0, op1, tmax, diff);
>       }
> 
> +    /* If diff == 0, the address difference is less than ESIZE,
> +       so all elements are safe from conflicts. */
> +    tcg_gen_movcond_i64(TCG_COND_EQ, diff, diff, tcg_constant_i64(0), tmax, diff);
> +
>       /* Bound to the maximum.  */
>       tcg_gen_umin_i64(diff, diff, tmax);
> 

For whilerw, we should replace the preceding test, which for some reason compares op0 == 
op1 directly instead of diff == 0 after the division.

For whilewr, we need 65-bit arithmetic for the straightforward solution, which I think 
means we need two comparisons, including this one.  Possibly clearer as

     diff = unsigned_saturating_sub(op1, op0)
     diff = diff >> esz
     diff = diff ? diff : max
     diff = umin(diff, max)

where unsigned_saturating_sub(x, y) -> umax(x, y) - y

Less clear, but possibly more efficient,

     diff = umin(diff - 1, max - 1) + 1;

instead of the direct comparison against 0.

I'll prepare a patch.  Thanks for the report.


r~