Re: [PATCH 2/2] target/i386: fix IEEE x87 floating-point exception raising

[Richard Henderson]

On 5/15/20 2:21 PM, Joseph Myers wrote:
> +    uint8_t new_flags = get_float_exception_flags(&env->fp_status);
> +    float_raise(old_flags, &env->fp_status);
> +    fpu_set_exception(env,
> +                      ((new_flags & float_flag_invalid ? FPUS_IE : 0) |
> +                       (new_flags & float_flag_divbyzero ? FPUS_ZE : 0) |
> +                       (new_flags & float_flag_overflow ? FPUS_OE : 0) |
> +                       (new_flags & float_flag_underflow ? FPUS_UE : 0) |
> +                       (new_flags & float_flag_inexact ? FPUS_PE : 0) |
> +                       (new_flags & float_flag_input_denormal ? FPUS_DE : 
> 0)));
> +}

This is not ideal from the point of view of interacting with softfloat's
deferral to host hard float.

I know you're working toward raising unmasked exceptions, but I think we will
want to handle that in a different way.

To retain the hard float fast path, we need to leave float_flag_invalid set
when the accrued exception bit is set.  To me this suggests keep all of the
FPUS_* bits in fp_status and only convert to FPUS_* when we read the fp status
word.

When it comes to raising unmasked exceptions... I have a couple of thoughts.

(1) Enhance softfloat to record the exceptions raised for the previous
operation, separate from the accrued exceptions.

This gets into trouble vs float_flag_invalid though, because we can't compute
that through the hard-float fast path.  And without knowing that
float_flag_invalid is currently irrelevant, we can't use the fast path at all.

This might still help in some places (though not here), so it might still be
worth exploring.

(2) In save_exception_flags, only zero the bits for which we have unmasked
exceptions.  In the normal case this would be none of them, which solves the
fast-path problem above.

To simplify this at runtime, I would suggest pre-computing a softfloat mask of
unmasked exceptions when we change the fp control word.


r~