| Message ID | 20240123153416.877308-1-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Fix A64 scalar SQSHRN and SQRSHRN | expand |
On 1/24/24 01:34, Peter Maydell wrote: > In commit 1b7bc9b5c8bf374dd we changed handle_vec_simd_sqshrn() so > that instead of starting with a 0 value and depositing in each new > element from the narrowing operation, it instead started with the raw > result of the narrowing operation of the first element. > > This is fine in the vector case, because the deposit operations for > the second and subsequent elements will always overwrite any higher > bits that might have been in the first element's result value in > tcg_rd. However in the scalar case we only go through this loop > once. The effect is that for a signed narrowing operation, if the > result is negative then we will now return a value where the bits > above the first element are incorrectly 1 (because the narrowfn > returns a sign-extended result, not one that is truncated to the > element size). > > Fix this by using an extract operation to get exactly the correct > bits of the output of the narrowfn for element 1, instead of a > plain move. > > Cc:qemu-stable@nongnu.org > Fixes: 1b7bc9b5c8bf374dd3 ("target/arm: Avoid tcg_const_ptr in handle_vec_simd_sqshrn") > Resolves:https://gitlab.com/qemu-project/qemu/-/issues/2089 > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/tcg/translate-a64.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index 27335e85407..340265beb05 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -8343,7 +8343,7 @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool is_scalar, bool is_q, narrowfn(tcg_rd_narrowed, tcg_env, tcg_rd); tcg_gen_extu_i32_i64(tcg_rd, tcg_rd_narrowed); if (i == 0) { - tcg_gen_mov_i64(tcg_final, tcg_rd); + tcg_gen_extract_i64(tcg_final, tcg_rd, 0, esize); } else { tcg_gen_deposit_i64(tcg_final, tcg_final, tcg_rd, esize * i, esize); }
In commit 1b7bc9b5c8bf374dd we changed handle_vec_simd_sqshrn() so that instead of starting with a 0 value and depositing in each new element from the narrowing operation, it instead started with the raw result of the narrowing operation of the first element. This is fine in the vector case, because the deposit operations for the second and subsequent elements will always overwrite any higher bits that might have been in the first element's result value in tcg_rd. However in the scalar case we only go through this loop once. The effect is that for a signed narrowing operation, if the result is negative then we will now return a value where the bits above the first element are incorrectly 1 (because the narrowfn returns a sign-extended result, not one that is truncated to the element size). Fix this by using an extract operation to get exactly the correct bits of the output of the narrowfn for element 1, instead of a plain move. Cc: qemu-stable@nongnu.org Fixes: 1b7bc9b5c8bf374dd3 ("target/arm: Avoid tcg_const_ptr in handle_vec_simd_sqshrn") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2089 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/tcg/translate-a64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)