Re: [Qemu-devel] [PATCH v1 06/21] RISC-V FPU Support

[Richard Henderson]

On 01/23/2018 03:15 PM, Michael Clark wrote:
>     > +uint64_t helper_fmsub_s(CPURISCVState *env, uint64_t frs1, uint64_t 
> frs2,
>     > +                        uint64_t frs3, uint64_t rm)
>     > +{
>     > +    require_fp;
>     > +    set_float_rounding_mode(RM, &env->fp_status);
>     > +    frs1 = float32_muladd(frs1, frs2, frs3 ^ (uint32_t)INT32_MIN, 0,
>     > +                          &env->fp_status);
> 
>     Given that RISC-V always returns a default NaN, you obviously do not care 
> about
>     the sign of a NaN result.  Therefore you should use float_muladd_negate_c 
> as
>     the fourth argument here and not perform the sign flip manually.
> 
> 
> We do care about the sign of NaN results.
> 
> Jim Wilson spotted this bug and removed a call to set_default_nan_mode
> 
> https://github.com/riscv/riscv-qemu/commit/4223d89b0c5c671332d66bcd649db5c6f46559f5

Ok.  Now it depends on what result you care about for madd specifically.

If, like x86 and Power, fmsub returns the (silenced) original input NaN, you
want the float_muladd_* flags.

If, like ARM, fmsub returns the (silenced) negated input NaN, then you do need
to change sign externally.  If this is the case, please use float32_chs instead
of open-coding it with xor.


r~