On Fri, Mar 31, 2023 at 11:39 PM Richard Henderson <richard.hender...@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.hender...@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~