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

[Richard Henderson]

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.


r~