Re: [Qemu-ppc] [Qemu-devel] [PULL 06/37] target-ppc: VXSQRT Should Not Be Set for NaNs

[Tom Musta]

I agree that the comment is incorrect and should say "sNaN square root".

On Thu, Feb 12, 2015 at 4:21 PM, Maciej W. Rozycki <address@hidden> wrote:

On Wed, 7 Jan 2015, Alexander Graf wrote:

> diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
> index 7f74466..81db60f 100644
> --- a/target-ppc/fpu_helper.c
> +++ b/target-ppc/fpu_helper.c
> @@ -920,14 +923,16 @@ uint64_t helper_fsqrt(CPUPPCState *env, uint64_t arg)
>
>      farg.ll = arg;
>
> -    if (unlikely(float64_is_neg(farg.d) && !float64_is_zero(farg.d))) {
> -        /* Square root of a negative nonzero number */
> -        farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
> -    } else {
> +    if (unlikely(float64_is_any_nan(farg.d))) {
>          if (unlikely(float64_is_signaling_nan(farg.d))) {
> -            /* sNaN square root */
> +            /* sNaN reciprocal square root */

 This change to the comment looks accidental, compare the changes below.
Should it be reverted?  [Found this while resolving merge conflicts.]

>              fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> +            farg.ll = float64_snan_to_qnan(farg.ll);
>          }
> +    } else if (unlikely(float64_is_neg(farg.d) && !float64_is_zero(farg.d))) {
> +        /* Square root of a negative nonzero number */
> +        farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
> +    } else {
>          farg.d = float64_sqrt(farg.d, &env->fp_status);
>      }
>      return farg.ll;
> @@ -974,17 +979,20 @@ uint64_t helper_frsqrte(CPUPPCState *env, uint64_t arg)
>
>      farg.ll = arg;
>
> -    if (unlikely(float64_is_neg(farg.d) && !float64_is_zero(farg.d))) {
> -        /* Reciprocal square root of a negative nonzero number */
> -        farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
> -    } else {
> +    if (unlikely(float64_is_any_nan(farg.d))) {
>          if (unlikely(float64_is_signaling_nan(farg.d))) {
>              /* sNaN reciprocal square root */
>              fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> +            farg.ll = float64_snan_to_qnan(farg.ll);
>          }
> +    } else if (unlikely(float64_is_neg(farg.d) && !float64_is_zero(farg.d))) {
> +        /* Reciprocal square root of a negative nonzero number */
> +        farg.ll = fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
> +    } else {
>          farg.d = float64_sqrt(farg.d, &env->fp_status);
>          farg.d = float64_div(float64_one, farg.d, &env->fp_status);
>      }
> +
>      return farg.ll;
>  }
>

  Maciej