On Mon, Jan 10, 2011 at 06:14:18PM -0600, Peter Maydell wrote:
> On 2 January 2011 08:39, Aurelien Jarno <aurel...@aurel32.net> 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 <ag...@suse.de>
> > Signed-off-by: Aurelien Jarno <aurel...@aurel32.net>
>
> > @@ -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.)
It is actually a mistake, though the result is correct. I'll resend the
patch after fixing that, thanks for the review.
> > @@ -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.
>
Same here.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
