Re: [PATCH 1/2] target/i386: Do not raise Invalid for 0 * Inf + QNaN

[Richard Henderson <richard.henderson@linaro.org>]

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-stable@nongnu.org
> Fixes: 8adcff4ae7 (""fpu: handle raising Invalid for infzero in pick_nan_muladd")
> Signed-off-by: Peter Maydell <peter.maydell@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?

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~