qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: Joseph Myers <joseph@codesourcery.com>,
	qemu-devel@nongnu.org, pbonzini@redhat.com, rth@twiddle.net,
	ehabkost@redhat.com
Subject: Re: [PATCH 2/2] target/i386: fix IEEE x87 floating-point exception raising
Date: Tue, 19 May 2020 10:43:31 -0700	[thread overview]
Message-ID: <5dd2d81c-cedd-7835-6b3c-7e089254dc95@linaro.org> (raw)
In-Reply-To: <alpine.DEB.2.21.2005152120280.3469@digraph.polyomino.org.uk>

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~


  reply	other threads:[~2020-05-19 17:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 21:19 [PATCH 0/2] target/i386: x87 exceptions fixes Joseph Myers
2020-05-15 21:20 ` [PATCH 1/2] target/i386: fix fisttpl, fisttpll handling of out-of-range values Joseph Myers
2020-05-15 21:21 ` [PATCH 2/2] target/i386: fix IEEE x87 floating-point exception raising Joseph Myers
2020-05-19 17:43   ` Richard Henderson [this message]
2020-05-19 18:12     ` Joseph Myers
2020-05-19 18:24       ` Richard Henderson
2020-05-19 18:28         ` Joseph Myers
2021-05-20 17:38   ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5dd2d81c-cedd-7835-6b3c-7e089254dc95@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=ehabkost@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).