[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] target-ppc: fix sNaN propagation
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] target-ppc: fix sNaN propagation |
Date: |
Mon, 10 Jan 2011 18:14:18 -0600 |
On 2 January 2011 08:39, Aurelien Jarno <address@hidden> wrote:
> The current FPU code returns 0.0 if one of the operand is a
> signaling NaN and the VXSNAN exception is disabled.
>
> fload_invalid_op_excp() doesn't return a qNaN in case of a VXSNAN
> exception as the operand should be propagated instead of a new
> qNaN to be generated. Fix that by calling fload_invalid_op_excp()
> only for the exception generation (if enabled), and use the softfloat
> code to correctly compute the result.
>
> Cc: Alexander Graf <address@hidden>
> Signed-off-by: Aurelien Jarno <address@hidden>
> @@ -1410,10 +1418,10 @@ uint64_t helper_frsp (uint64_t arg)
> if (unlikely(float64_is_signaling_nan(farg.d))) {
> /* sNaN square root */
> farg.ll = fload_invalid_op_excp(POWERPC_EXCP_FP_VXSNAN);
> - } else {
> - f32 = float64_to_float32(farg.d, &env->fp_status);
> - farg.d = float32_to_float64(f32, &env->fp_status);
> }
> + f32 = float64_to_float32(farg.d, &env->fp_status);
> + farg.d = float32_to_float64(f32, &env->fp_status);
> +
> return farg.ll;
> }
Most of these changes are to ignoring the result from
fload_invalid_op_excp(POWERPC_EXCP_FP_VXSNAN),
but this one leaves it assigning a value to farg.ll -- is there
any reason for that? (It looks like the assignment gets
immediately overwritten by the assignment to farg.d later.)
> @@ -1460,11 +1468,11 @@ uint64_t helper_fres (uint64_t arg)
> if (unlikely(float64_is_signaling_nan(farg.d))) {
> /* sNaN reciprocal */
> farg.ll = fload_invalid_op_excp(POWERPC_EXCP_FP_VXSNAN);
> - } else {
> - farg.d = float64_div(float64_one, farg.d, &env->fp_status);
> - f32 = float64_to_float32(farg.d, &env->fp_status);
> - farg.d = float32_to_float64(f32, &env->fp_status);
> }
> + farg.d = float64_div(float64_one, farg.d, &env->fp_status);
> + f32 = float64_to_float32(farg.d, &env->fp_status);
> + farg.d = float32_to_float64(f32, &env->fp_status);
> +
> return farg.ll;
> }
>
Ditto for this hunk.
Looks plausible to me other than that.
-- PMM