qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] target-ppc: fix fcmp{o,u} instructions


From: Aurelien Jarno
Subject: Re: [Qemu-devel] [PATCH] target-ppc: fix fcmp{o,u} instructions
Date: Sun, 14 Dec 2008 20:34:51 +0100
User-agent: Mutt/1.5.18 (2008-05-17)

On Sun, Dec 14, 2008 at 11:10:46AM -0800, Nathan Froyd wrote:
> On Sun, Dec 14, 2008 at 11:35:54AM +0100, Aurelien Jarno wrote:
> > Please find my comments inline.
> 
> Aurelien also pointed out an error on IRC: we need to use real TCG
> temporaries for the crfD value, not the integers themselves.  Fixed
> thusly (sorry, not exactly sure how to do this cleanly for git's sake).
> 
> -Nathan
> 
> The instructions are specified to update the condition register even if
> an error is to be signaled because of NaN input.
> 
> Signed-off-by: Nathan Froyd <address@hidden>

Thanks, applied.

> ---
>  target-ppc/helper.h    |    4 +-
>  target-ppc/op_helper.c |   56 ++++++++++++++++++++++++++---------------------
>  target-ppc/translate.c |   12 ++++++---
>  3 files changed, 41 insertions(+), 31 deletions(-)
> 
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 26d1627..07d8f8a 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -62,8 +62,8 @@ DEF_HELPER_1(fpscr_setbit, void, i32)
>  DEF_HELPER_1(float64_to_float32, i32, i64)
>  DEF_HELPER_1(float32_to_float64, i64, i32)
>  
> -DEF_HELPER_2(fcmpo, i32, i64, i64)
> -DEF_HELPER_2(fcmpu, i32, i64, i64)
> +DEF_HELPER_3(fcmpo, void, i64, i64, i32)
> +DEF_HELPER_3(fcmpu, void, i64, i64, i32)
>  
>  DEF_HELPER_1(fctiw, i64, i64)
>  DEF_HELPER_1(fctiwz, i64, i64)
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index cd88bb6..c0b4c70 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -1602,32 +1602,36 @@ uint64_t helper_fsel (uint64_t arg1, uint64_t arg2, 
> uint64_t arg3)
>          return arg3;
>  }
>  
> -uint32_t helper_fcmpu (uint64_t arg1, uint64_t arg2)
> +void helper_fcmpu (uint64_t arg1, uint64_t arg2, uint32_t crfD)
>  {
>      CPU_DoubleU farg1, farg2;
>      uint32_t ret = 0;
>      farg1.ll = arg1;
>      farg2.ll = arg2;
>  
> -    if (unlikely(float64_is_signaling_nan(farg1.d) ||
> -                 float64_is_signaling_nan(farg2.d))) {
> -        /* sNaN comparison */
> -        fload_invalid_op_excp(POWERPC_EXCP_FP_VXSNAN);
> +    if (unlikely(float64_is_nan(farg1.d) ||
> +                 float64_is_nan(farg2.d))) {
> +        ret = 0x01UL;
> +    } else if (float64_lt(farg1.d, farg2.d, &env->fp_status)) {
> +        ret = 0x08UL;
> +    } else if (!float64_le(farg1.d, farg2.d, &env->fp_status)) {
> +        ret = 0x04UL;
>      } else {
> -        if (float64_lt(farg1.d, farg2.d, &env->fp_status)) {
> -            ret = 0x08UL;
> -        } else if (!float64_le(farg1.d, farg2.d, &env->fp_status)) {
> -            ret = 0x04UL;
> -        } else {
> -            ret = 0x02UL;
> -        }
> +        ret = 0x02UL;
>      }
> +
>      env->fpscr &= ~(0x0F << FPSCR_FPRF);
>      env->fpscr |= ret << FPSCR_FPRF;
> -    return ret;
> +    env->crf[crfD] = ret;
> +    if (unlikely(ret == 0x01UL
> +                 && (float64_is_signaling_nan(farg1.d) ||
> +                     float64_is_signaling_nan(farg2.d)))) {
> +        /* sNaN comparison */
> +        fload_invalid_op_excp(POWERPC_EXCP_FP_VXSNAN);
> +    }
>  }
>  
> -uint32_t helper_fcmpo (uint64_t arg1, uint64_t arg2)
> +void helper_fcmpo (uint64_t arg1, uint64_t arg2, uint32_t crfD)
>  {
>      CPU_DoubleU farg1, farg2;
>      uint32_t ret = 0;
> @@ -1636,6 +1640,19 @@ uint32_t helper_fcmpo (uint64_t arg1, uint64_t arg2)
>  
>      if (unlikely(float64_is_nan(farg1.d) ||
>                   float64_is_nan(farg2.d))) {
> +        ret = 0x01UL;
> +    } else if (float64_lt(farg1.d, farg2.d, &env->fp_status)) {
> +        ret = 0x08UL;
> +    } else if (!float64_le(farg1.d, farg2.d, &env->fp_status)) {
> +        ret = 0x04UL;
> +    } else {
> +        ret = 0x02UL;
> +    }
> +
> +    env->fpscr &= ~(0x0F << FPSCR_FPRF);
> +    env->fpscr |= ret << FPSCR_FPRF;
> +    env->crf[crfD] = ret;
> +    if (unlikely (ret == 0x01UL)) {
>          if (float64_is_signaling_nan(farg1.d) ||
>              float64_is_signaling_nan(farg2.d)) {
>              /* sNaN comparison */
> @@ -1645,18 +1662,7 @@ uint32_t helper_fcmpo (uint64_t arg1, uint64_t arg2)
>              /* qNaN comparison */
>              fload_invalid_op_excp(POWERPC_EXCP_FP_VXVC);
>          }
> -    } else {
> -        if (float64_lt(farg1.d, farg2.d, &env->fp_status)) {
> -            ret = 0x08UL;
> -        } else if (!float64_le(farg1.d, farg2.d, &env->fp_status)) {
> -            ret = 0x04UL;
> -        } else {
> -            ret = 0x02UL;
> -        }
>      }
> -    env->fpscr &= ~(0x0F << FPSCR_FPRF);
> -    env->fpscr |= ret << FPSCR_FPRF;
> -    return ret;
>  }
>  
>  #if !defined (CONFIG_USER_ONLY)
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 69801d0..17e5fd7 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -2269,26 +2269,30 @@ GEN_FLOAT_B(rim, 0x08, 0x0F, 1, PPC_FLOAT_EXT);
>  /* fcmpo */
>  GEN_HANDLER(fcmpo, 0x3F, 0x00, 0x01, 0x00600001, PPC_FLOAT)
>  {
> +    TCGv crf;
>      if (unlikely(!ctx->fpu_enabled)) {
>          gen_exception(ctx, POWERPC_EXCP_FPU);
>          return;
>      }
>      gen_reset_fpstatus();
> -    gen_helper_fcmpo(cpu_crf[crfD(ctx->opcode)],
> -                     cpu_fpr[rA(ctx->opcode)], cpu_fpr[rB(ctx->opcode)]);
> +    crf = tcg_const_i32(crfD(ctx->opcode));
> +    gen_helper_fcmpo(cpu_fpr[rA(ctx->opcode)], cpu_fpr[rB(ctx->opcode)], 
> crf);
> +    tcg_temp_free(crf);
>      gen_helper_float_check_status();
>  }
>  
>  /* fcmpu */
>  GEN_HANDLER(fcmpu, 0x3F, 0x00, 0x00, 0x00600001, PPC_FLOAT)
>  {
> +    TCGv crf;
>      if (unlikely(!ctx->fpu_enabled)) {
>          gen_exception(ctx, POWERPC_EXCP_FPU);
>          return;
>      }
>      gen_reset_fpstatus();
> -    gen_helper_fcmpu(cpu_crf[crfD(ctx->opcode)],
> -                     cpu_fpr[rA(ctx->opcode)], cpu_fpr[rB(ctx->opcode)]);
> +    crf = tcg_const_i32(crfD(ctx->opcode));
> +    gen_helper_fcmpu(cpu_fpr[rA(ctx->opcode)], cpu_fpr[rB(ctx->opcode)], 
> crf);
> +    tcg_temp_free(crf);
>      gen_helper_float_check_status();
>  }
>  
> -- 
> 1.6.0.5
> 
> 
> 
> 

-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net




reply via email to

[Prev in Thread] Current Thread [Next in Thread]