Re: [Qemu-devel] [PATCH v2] PPC: Depend behavior of cmp instructions only on instruction encoding

[Aurelien Jarno <aurelien@aurel32.net>]

On Wed, May 08, 2013 at 08:21:56PM +0200, Alexander Graf wrote:
> When running an L=1 cmp instruction on a 64bit PPC CPU with SF off, it
> still behaves identical to what it does when SF is on. Remove the implicit
> difference in the code.
> 
> Also, on most 32bit CPUs we should always treat the compare as 32bit
> compare, as the CPU will ignore the L bit. This is not true for e500mc,
> but that's up for a different patch.
> 
> Reported-by: Torbjorn Granlund <tg@gmplib.org>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - ignore L bit
> ---
>  target-ppc/translate.c |   32 ++++++++++++++++----------------
>  1 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a018616..4590c6f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -675,48 +675,48 @@ static inline void gen_set_Rc0(DisasContext *ctx, TCGv reg)
>  /* cmp */
>  static void gen_cmp(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> -        gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                     1, crfD(ctx->opcode));
> -    } else {
> +    if ((ctx->opcode & 0x00200000) && (ctx->insns_flags & PPC_64B)) {
>          gen_op_cmp(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                     1, crfD(ctx->opcode));
> +    } else {
> +        gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> +                     1, crfD(ctx->opcode));
>      }
>  }
>  
>  /* cmpi */
>  static void gen_cmpi(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> -        gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
> -                      1, crfD(ctx->opcode));
> -    } else {
> +    if ((ctx->opcode & 0x00200000) && (ctx->insns_flags & PPC_64B)) {
>          gen_op_cmpi(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
>                      1, crfD(ctx->opcode));
> +    } else {
> +        gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
> +                      1, crfD(ctx->opcode));
>      }
>  }
>  
>  /* cmpl */
>  static void gen_cmpl(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> -        gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> -                     0, crfD(ctx->opcode));
> -    } else {
> +    if ((ctx->opcode & 0x00200000) && (ctx->insns_flags & PPC_64B)) {
>          gen_op_cmp(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                     0, crfD(ctx->opcode));
> +    } else {
> +        gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
> +                     0, crfD(ctx->opcode));
>      }
>  }
>  
>  /* cmpli */
>  static void gen_cmpli(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> -        gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
> -                      0, crfD(ctx->opcode));
> -    } else {
> +    if ((ctx->opcode & 0x00200000) && (ctx->insns_flags & PPC_64B)) {
>          gen_op_cmpi(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
>                      0, crfD(ctx->opcode));
> +    } else {
> +        gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
> +                      0, crfD(ctx->opcode));
>      }
>  }
>  

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net