Re: [Qemu-devel] [PATCH] tcg: correct 32-bit tcg_gen_ld8s_i64 sign-extension

[Peter Maydell]

On 28 October 2016 at 00:35, Joseph Myers <address@hidden> wrote:
> The version of tcg_gen_ld8s_i64 for 32-bit systems does a load into
> the low part of the return value - then attempts a sign extension into
> the high part, but wrongly sets the high part to a sign extension of
> itself rather than of the low part.  This results in TCG internal
> errors from the use of the uninitialized high part (in some GCC tests
> of AArch64 NEON shift intrinsics, in particular).  This patch corrects
> the sign-extension logic, making it match other functions such as
> tcg_gen_ld16s_i64.
>
> Signed-off-by: Joseph Myers <address@hidden>
>
> ---
>
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index bb2bfee..43d34ea 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -790,7 +790,7 @@ void tcg_gen_ld8u_i64(TCGv_i64 ret, TCGv_ptr arg2, 
> tcg_target_long offset)
>  void tcg_gen_ld8s_i64(TCGv_i64 ret, TCGv_ptr arg2, tcg_target_long offset)
>  {
>      tcg_gen_ld8s_i32(TCGV_LOW(ret), arg2, offset);
> -    tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_HIGH(ret), 31);
> +    tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_LOW(ret), 31);
>  }
>
>  void tcg_gen_ld16u_i64(TCGv_i64 ret, TCGv_ptr arg2, tcg_target_long offset)

Reviewed-by: Peter Maydell <address@hidden>

As far as I can tell this bug was introduced in commit a7812ae4123
in 2008, which is a remarkably long time for a basic bug like this
to persist...

thanks
-- PMM