On Thu, 16 Jan 2025 at 15:22, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 1/16/25 03:25, Peter Maydell wrote:
> > In commit 8adcff4ae7 ("fpu: handle raising Invalid for infzero in
> > pick_nan_muladd") we changed the handling of 0 * Inf + QNaN to always
> > raise the Invalid exception regardless of target architecture. (This
> > was a change affecting hppa, i386, sh4 and tricore.) However, this
> > was incorrect for i386, which documents in the SDM section 14.5.2
> > that for the 0 * Inf + NaN case that it will only raise the Invalid
> > exception when the input is an SNaN. (This is permitted by the IEEE
> > 754-2008 specification, which documents that whether we raise Invalid
> > for 0 * Inf + QNaN is implementation defined.)
> >
> > Adjust the softfloat pick_nan_muladd code to allow the target to
> > suppress the raising of Invalid for the inf * zero + NaN case (as an
> > extra flag orthogonal to its choice for when to use the default NaN),
> > and enable that for x86.
> >
> > We do not revert here the behaviour change for hppa, sh4 or tricore:
> > * The sh4 manual is clear that it should signal Invalid
> > * The tricore manual is a bit vague but doesn't say it shouldn't
> > * The hppa manual doesn't talk about fused multiply-add corner
> > cases at all
> >
> > Cc: qemu-sta...@nongnu.org
> > Fixes: 8adcff4ae7 (""fpu: handle raising Invalid for infzero in
> > pick_nan_muladd")
> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> > ---
> > include/fpu/softfloat-types.h | 16 +++++++++++++---
> > target/i386/tcg/fpu_helper.c | 5 ++++-
> > fpu/softfloat-parts.c.inc | 5 +++--
> > 3 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
> > index 9d37cdfaa8e..c51b2a5b3de 100644
> > --- a/include/fpu/softfloat-types.h
> > +++ b/include/fpu/softfloat-types.h
> > @@ -278,11 +278,21 @@ typedef enum __attribute__((__packed__)) {
> > /* No propagation rule specified */
> > float_infzeronan_none = 0,
> > /* Result is never the default NaN (so always the input NaN) */
> > - float_infzeronan_dnan_never,
> > + float_infzeronan_dnan_never = 1,
> > /* Result is always the default NaN */
> > - float_infzeronan_dnan_always,
> > + float_infzeronan_dnan_always = 2,
> > /* Result is the default NaN if the input NaN is quiet */
> > - float_infzeronan_dnan_if_qnan,
> > + float_infzeronan_dnan_if_qnan = 3,
> > + /*
> > + * Don't raise Invalid for 0 * Inf + NaN. Default is to raise.
> > + * IEEE 754-2008 section 7.2 makes it implementation defined whether
> > + * 0 * Inf + QNaN raises Invalid or not. Note that 0 * Inf + SNaN will
> > + * raise the Invalid flag for the SNaN anyway.
> > + *
> > + * This is a flag which can be ORed in with any of the above
> > + * DNaN behaviour options.
> > + */
> > + float_infzeronan_suppress_invalid = (1 << 7),
>
> Why 128 and not 4?
I wanted to leave space for adding possible future
values to the dnan options without having to renumber
the suppress_invalid flag. So I put it at the top of
an 8 bit value. But I can use 4 if you prefer.
thanks
-- PMM
