Re: [PATCH 24/76] fpu: allow flushing of output denormals to be after rounding

[Peter Maydell]

On Sat, 25 Jan 2025 at 16:42, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/24/25 08:27, Peter Maydell wrote:
> > Currently we handle flushing of output denormals in uncanon_normal
> > always before we deal with rounding.  This works for architectures
> > that detect tininess before rounding, but is usually not the right
> > place when the architecture detects tininess after rounding.  For
> > example, for x86 the SDM states that the MXCSR FTZ control bit causes
> > outputs to be flushed to zero "when it detects a floating-point
> > underflow condition".  This means that we mustn't flush to zero if
> > the input is such that after rounding it is no longer tiny.
> >
> > At least one of our guest architectures does underflow detection
> > after rounding but flushing of denormals before rounding (MIPS MSA);
>
> Whacky, but yes, I see that in the msa docs.
>
> > Add an ftz_detection flag.  For consistency with
> > tininess_before_rounding, we make it default to "detect ftz after
> > rounding"; this means that we need to explicitly set the flag to
> > "detect ftz before rounding" on every existing architecture that sets
> > flush_to_zero, so that this commit has no behaviour change.
> > (This means more code change here but for the long term a less
> > confusing API.)
>
> Do we really want flush_to_zero to be separate from ftz_detection?
>
> E.g.
>
> enum {
>    float_ftz_disabled,
>    float_ftz_after_rounding,
>    float_ftz_before_rounding,
> }

I did consider that, but on almost all targets the "before
or after rounding" setting is constant for the life of the
emulation, whereas turning ftz on and off via a status register
bit is common. I preferred to leave it so that you could continue
to write:
 set_flush_to_zero((vscr >> VSCR_NJ) & 1, &env->vec_status)
or whatever, rather than having to switch to
  (vscr >> VSCR_NJ) ? float_ftz_before_rounding : float_ftz_disabled.
which in addition to being more longwinded also means that the
"is this architecture ftz before or after rounding" setting is
scattered in multiple places, wherever it turns FTZ on or off.
And for Arm it gets more awkward, because the FZ bit is
"turn FTZ on or off, whatever its current semantics are", so
you end up needing "FZ ? AH ? after_rounding : before_rounding : disabled".

Keeping the on/off and the "what semantics is your architecture"
separate questions I think is simpler.

> BTW, I'm not keen on your "detect_*" names, without "float_" prefix like 
> (almost?)
> everything else.

Yes, I'm not super enthused about them either. Happy to switch
to something else. We're not very consistent about 'float',
though: eg set_flush_to_zero, set_snan_bit_is_one,
set_flush_inputs_to_zero.

-- PMM