Re: [Qemu-devel] [Qemu-ppc] Fwd: Patch: fix to gen_mcrxr() in target-ppc/translate.c

[Tom Musta <tommusta@gmail.com>]

On 6/11/2014 9:23 AM, Sorav Bansal wrote:
>> Please read my comments again.  I agree that SO, OV and CA are stored in the LSB of their internal QEMU representation.  The problem is that, even with your patch, these bits are not being correctly shifted into the target four bit CR field.
>>
> 
> Ah, I forgot that PPC spec starts bit numbering at the MSB. Here is
> the revised patch.
> 
> 
> From da0a962a6d14fe699ebb7cc12450c7de9553b66a Mon Sep 17 00:00:00 2001
> From: Sorav Bansal <sbansal@cse.iitd.ernet.in>
> Date: Wed, 11 Jun 2014 19:49:49 +0530
> Subject: [PATCH] Fixed the translation of the mcrxr ppc instruction
> 
> ---
>  target-ppc/translate.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index f089014..59a92b9 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4147,8 +4147,9 @@ static void gen_mcrxr(DisasContext *ctx)
>      tcg_gen_trunc_tl_i32(t0, cpu_so);
>      tcg_gen_trunc_tl_i32(t1, cpu_ov);
>      tcg_gen_trunc_tl_i32(dst, cpu_ca);
> -    tcg_gen_shri_i32(t0, t0, 2);
> -    tcg_gen_shri_i32(t1, t1, 1);
> +    tcg_gen_shli_i32(t0, t0, 3);
> +    tcg_gen_shli_i32(t1, t1, 2);
> +    tcg_gen_shli_i32(dst, dst, 1);
>      tcg_gen_or_i32(dst, dst, t0);
>      tcg_gen_or_i32(dst, dst, t1);
>      tcg_temp_free_i32(t0);
> 

Your patch is missing the signoff.

Other than that, this does properly shift into the CR field.  My other comments about using deposit and lack of handling XER[35] still stand, but I don't believe your patch makes things worse.

Reviewed-by: Tom Musta <tommusta@gmail.com>