Re: [Qemu-devel] [V2 PATCH 1/8] target-ppc: Bug Fix: rlwinm

[Richard Henderson <rth@twiddle.net>]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> The rlwinm specification includes the ROTL32 operation, which is defined
> to be a left rotation of two copies of the least significant 32 bits of
> the source GPR.
> 
> The current implementation is incorrect on 64-bit implementations in that
> it rotates a single copy of the least significant 32 bits, padding with
> zeroes in the most significant bits.
> 
> Fix the code to properly implement this ROTL32 operation.
> 
> Example:
> R3 = F7487D82EC6F75DF
> rlwinm 3,3,5,12,4
> 
> R3 expected : 8DEEBBFD880EBBFD
> R3 actual   : 00000000880EBBFD (without this fix)
> 
> Signed-off-by: Tom Musta <tommusta@gmail.com>
> ---
>  target-ppc/translate.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index b23933f..a27d063 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1672,11 +1672,9 @@ static void gen_rlwinm(DisasContext *ctx)
>      } else {
>          TCGv t0 = tcg_temp_new();
>  #if defined(TARGET_PPC64)
> -        TCGv_i32 t1 = tcg_temp_new_i32();
> -        tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
> -        tcg_gen_rotli_i32(t1, t1, sh);
> -        tcg_gen_extu_i32_i64(t0, t1);
> -        tcg_temp_free_i32(t1);
> +        tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)],
> +            cpu_gpr[rS(ctx->opcode)], 32, 32);
> +        tcg_gen_rotli_i64(t0, t0, sh);
>  #else
>          tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh);
>  #endif
> 

You might want to retain a special case for mb==0 and me==31 which would be a
straight 32-bit rotate with subsequent zero-extension.  Just like we've got
special cases for the normal shifts.


r~