From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44533)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <macro@linux-mips.org>) id 1YM28O-0005kd-LH
	for qemu-devel@nongnu.org; Thu, 12 Feb 2015 17:21:21 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <macro@linux-mips.org>) id 1YM28J-0002Tg-Uk
	for qemu-devel@nongnu.org; Thu, 12 Feb 2015 17:21:20 -0500
Received: (from localhost user: 'macro', uid#1010) by eddie.linux-mips.org
	with ESMTP id S27013352AbbBLWVIkpHKH (ORCPT
	<rfc822;qemu-devel@nongnu.org> + 1 other);
	Thu, 12 Feb 2015 23:21:08 +0100
Date: Thu, 12 Feb 2015 22:21:08 +0000 (GMT)
Sender: "Maciej W. Rozycki" <macro@linux-mips.org>
From: "Maciej W. Rozycki" <macro@linux-mips.org>
In-Reply-To: <1420644048-16919-7-git-send-email-agraf@suse.de>
Message-ID: <alpine.LFD.2.11.1502122217460.22715@eddie.linux-mips.org>
References: <1420644048-16919-1-git-send-email-agraf@suse.de>
	<1420644048-16919-7-git-send-email-agraf@suse.de>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Subject: Re: [Qemu-devel] [PULL 06/37] target-ppc: VXSQRT Should Not Be Set
 for NaNs
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexander Graf <agraf@suse.de>
Cc: peter.maydell@linaro.org, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Tom Musta <tommusta@gmail.com>

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