On Wed, Oct 10, 2012 at 01:09:49PM -0700, Richard Henderson wrote:
> On 10/09/2012 01:27 PM, Aurelien Jarno wrote:
> > softfloat already has a few constants defined, use them instead of
> > redefining them in target-mips.
> >
> > Rename FLOAT_SNAN32 and FLOAT_SNAN64 to FP_TO_INT32_OVERFLOW and
> > FP_TO_INT64_OVERFLOW as even if they have the same value, they are
> > technically different (and defined differently in the MIPS ISA).
> >
> > Remove the unused constants.
> >
> > Signed-off-by: Aurelien Jarno <aurel...@aurel32.net>
>
> Reviewed-by: Richard Henderson <r...@twiddle.net>
>
> > @@ -2495,8 +2491,9 @@ uint64_t helper_float_cvtl_d(CPUMIPSState *env,
> > uint64_t fdt0)
> > set_float_exception_flags(0, &env->active_fpu.fp_status);
> > dt2 = float64_to_int64(fdt0, &env->active_fpu.fp_status);
> > update_fcr31(env);
> > - if (GET_FP_CAUSE(env->active_fpu.fcr31) & (FP_OVERFLOW | FP_INVALID))
> > - dt2 = FLOAT_SNAN64;
> > + if (GET_FP_CAUSE(env->active_fpu.fcr31) & (FP_OVERFLOW | FP_INVALID)) {
> > + dt2 = FP_TO_INT64_OVERFLOW;
> > + }
> > return dt2;
>
> That said, the existing code you're patching is incorrect.
>
> This code will fold to OVERFLOW if any previous operation caused an overflow,
> not checking that the *current* operation caused an overflow.
>
While I agree it should check the softfloat flags instead, I disagree it
is wrong. The part that GET_FP_CAUSE() is looking at is not the
accumulated flags, but the flags for the last instruction.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
