Re: [PATCH] riscv: Add support for the Zfa extension

[Christoph Müllner]

On Fri, Mar 31, 2023 at 11:39 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/31/23 11:22, Christoph Müllner wrote:
> > On Mon, Mar 27, 2023 at 7:18 PM Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 3/27/23 01:00, Christoph Muellner wrote:
> >>> +uint64_t helper_fminm_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
> >>> +{
> >>> +    float32 frs1 = check_nanbox_s(env, rs1);
> >>> +    float32 frs2 = check_nanbox_s(env, rs2);
> >>> +
> >>> +    if (float32_is_any_nan(frs1) || float32_is_any_nan(frs2)) {
> >>> +        return float32_default_nan(&env->fp_status);
> >>> +    }
> >>> +
> >>> +    return nanbox_s(env, float32_minimum_number(frs1, frs2, 
> >>> &env->fp_status));
> >>> +}
> >>
> >> Better to set and clear fp_status->default_nan_mode around the operation.
> >
> > I don't see how this can help:
> > * default_nan_mode defines if the default_nan is generated or if the
> > operand's NaN should be used
> > * RISC-V has default_nan_mode always set to true (operations should
> > return the a canonical NaN and not propagate NaN values)
> > * That also does not help to eliminate the is_any_nan() tests, because
> > float*_minimum_number() and float*_minnum() return the non-NaN number
> > if (only) one operand is NaN
> >
> > Am I missing something?
>
> Oh goodness, I did mis-read this.
>
> But if you need a nan when an input is a nan, then float32_min instead of
> float32_minimum_number (which goes out of its way to select the non-nan 
> result) is the
> correct function to use.

Understood and fixed.
Thanks!

>
>
> r~