Re: [Qemu-devel] [PATCH 7/7] target-mips: Add IEEE 754-2008 features support

[Maciej W. Rozycki]

On Mon, 9 Feb 2015, Leon Alrae wrote:

> > +        if (info->elf_flags & EF_MIPS_NAN2008)
> > +            env->active_fpu.fcr31 |=
> > +                (1 << FCR31_NAN2008) & env->active_fpu.fcr31_rw_bitmask;
> > +        else
> > +            env->active_fpu.fcr31 &=
> > +                ~((1 << FCR31_NAN2008) & env->active_fpu.fcr31_rw_bitmask);
> 
> braces are needed here

 Umm...

> > +uint32_t helper_float_chs_s(CPUMIPSState *env, uint32_t fst0)
> > +{
> > +    uint32_t fst1;
> > +
> > +    fst1 = float32_sub(0, fst0, &env->active_fpu.fp_status);
> > +    update_fcr31(env, GETPC());
> > +    return fst1;
> > +}
> 
> I think there is one case where helper_float_chs_{d,s,ps} are not
> correct -- when we have zero. In this case in subFloat32Sigs() we call:
> 
> return packFloat32(status->float_rounding_mode == float_round_down, 0, 0);
> 
> and the packFloat32() definition:
> 
> static inline float32 packFloat32(flag zSign, int_fast16_t zExp,
> uint32_t zSig)
> {
> 
>     return make_float32(
>           ( ( (uint32_t) zSign )<<31 ) + ( ( (uint32_t) zExp )<<23 ) +
> zSig);
> 
> }
> 
> Which means that the sign may not get changed, whereas I believe NEG.fmt
> is supposed to reverse the sign bit of positive/negative zero regardless
> of rounding mode.

 Good catch, I missed this corner case, thanks!  Another corner case is 
then helper_float_abs_{d,s,ps} with -0 as input and `float_round_down' 
being the current rounding mode.

 These cases could be addressed by either replacing subtraction from 0.0 
with multiplication by -1.0, or by tweaking the rounding mode as needed 
temporarily.  Given that the computational cost of multiplication is 
uncertain and likely higher or at best the same as the cost of addition or 
subtraction, I'd be leaning towards the latter solution.

 Thoughts?

  Maciej