From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Cc: Bruno Haible <bruno@clisp.org>
Subject: [PATCH 3/3] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990)
Date: Mon, 23 Oct 2023 18:36:59 -0300 [thread overview]
Message-ID: <20231023213659.3236496-4-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <20231023213659.3236496-1-adhemerval.zanella@linaro.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 6695 bytes --]
From: Bruno Haible <bruno@clisp.org>
According to ISO C23 (7.6.4.5), fesetexceptflag is supposed to set
floating-point exception flags without raising a trap.
The flags can be set in the 387 unit or in the SSE unit. When we need
to clear a flag, we need to do so in both units, due to the way
fetestexcept is implemented.
When we need to set a flag, it is sufficient to do it in the SSE unit,
because that is guaranteed to not trap. However, on i386 CPUs that have
only a 387 unit, set the flags in the 387, as long as this cannot trap.
Co-authored-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
math/test-fexcept-traps.c | 11 ++++++
sysdeps/i386/fpu/fsetexcptflg.c | 58 ++++++++++++++++++++-----------
sysdeps/x86_64/fpu/fsetexcptflg.c | 24 +++++++------
3 files changed, 62 insertions(+), 31 deletions(-)
diff --git a/math/test-fexcept-traps.c b/math/test-fexcept-traps.c
index 9b8f583ae6..a486d17951 100644
--- a/math/test-fexcept-traps.c
+++ b/math/test-fexcept-traps.c
@@ -19,6 +19,7 @@
#include <fenv.h>
#include <stdio.h>
#include <math-tests.h>
+#include <math-barriers.h>
static int
do_test (void)
@@ -67,6 +68,16 @@ do_test (void)
where setting the exception might result in traps the function should
return a nonzero value. */
ret = fesetexceptflag (&saved, FE_ALL_EXCEPT);
+
+ /* Execute some floating-point operations, since on some CPUs exceptions
+ triggers a trap only at the next floating-point instruction. */
+ double a = 1.0;
+ double b = a + a;
+ math_force_eval (b);
+ long double al = 1.0L;
+ long double bl = al + al;
+ math_force_eval (bl);
+
if (ret != 0 && !EXCEPTION_SET_FORCES_TRAP)
{
puts ("fesetexceptflag failed");
diff --git a/sysdeps/i386/fpu/fsetexcptflg.c b/sysdeps/i386/fpu/fsetexcptflg.c
index e724b7d6fd..ccbcf35e8e 100644
--- a/sysdeps/i386/fpu/fsetexcptflg.c
+++ b/sysdeps/i386/fpu/fsetexcptflg.c
@@ -17,42 +17,58 @@
<https://www.gnu.org/licenses/>. */
#include <fenv.h>
-#include <math.h>
-#include <unistd.h>
#include <ldsodefs.h>
-#include <dl-procinfo.h>
int
__fesetexceptflag (const fexcept_t *flagp, int excepts)
{
- fenv_t temp;
+ /* The flags can be set in the 387 unit or in the SSE unit. When we need to
+ clear a flag, we need to do so in both units, due to the way fetestexcept
+ is implemented.
+ When we need to set a flag, it is sufficient to do it in the SSE unit,
+ because that is guaranteed to not trap. However, on i386 CPUs that have
+ only a 387 unit, set the flags in the 387, as long as this cannot trap. */
- /* Get the current environment. We have to do this since we cannot
- separately set the status word. */
- __asm__ ("fnstenv %0" : "=m" (*&temp));
+ fenv_t temp;
- temp.__status_word &= ~(excepts & FE_ALL_EXCEPT);
- temp.__status_word |= *flagp & excepts & FE_ALL_EXCEPT;
+ excepts &= FE_ALL_EXCEPT;
- /* Store the new status word (along with the rest of the environment.
- Possibly new exceptions are set but they won't get executed unless
- the next floating-point instruction. */
- __asm__ ("fldenv %0" : : "m" (*&temp));
+ /* Get the current x87 FPU environment. We have to do this since we
+ cannot separately set the status word. */
+ __asm__ ("fnstenv %0" : "=m" (*&temp));
- /* If the CPU supports SSE, we set the MXCSR as well. */
if (CPU_FEATURE_USABLE (SSE))
{
- unsigned int xnew_exc;
+ unsigned int mxcsr;
+
+ /* Clear relevant flags. */
+ temp.__status_word &= ~(excepts & ~ *flagp);
- /* Get the current MXCSR. */
- __asm__ ("stmxcsr %0" : "=m" (*&xnew_exc));
+ /* Store the new status word (along with the rest of the environment). */
+ __asm__ ("fldenv %0" : : "m" (*&temp));
- /* Set the relevant bits. */
- xnew_exc &= ~(excepts & FE_ALL_EXCEPT);
- xnew_exc |= *flagp & excepts & FE_ALL_EXCEPT;
+ /* And now similarly for SSE. */
+ __asm__ ("stmxcsr %0" : "=m" (*&mxcsr));
+
+ /* Clear or set relevant flags. */
+ mxcsr ^= (mxcsr ^ *flagp) & excepts;
/* Put the new data in effect. */
- __asm__ ("ldmxcsr %0" : : "m" (*&xnew_exc));
+ __asm__ ("ldmxcsr %0" : : "m" (*&mxcsr));
+ }
+ else
+ {
+ /* Clear or set relevant flags. */
+ temp.__status_word ^= (temp.__status_word ^ *flagp) & excepts;
+
+ if ((~temp.__control_word) & temp.__status_word & excepts)
+ /* Setting the exception flags may trigger a trap (at the next
+ floating-point instruction, but that does not matter).
+ ISO C 23 § 7.6.4.5 does not allow it. */
+ return -1;
+
+ /* Store the new status word (along with the rest of the environment). */
+ __asm__ ("fldenv %0" : : "m" (*&temp));
}
/* Success. */
diff --git a/sysdeps/x86_64/fpu/fsetexcptflg.c b/sysdeps/x86_64/fpu/fsetexcptflg.c
index a3ac1dea01..2ce2b509f2 100644
--- a/sysdeps/x86_64/fpu/fsetexcptflg.c
+++ b/sysdeps/x86_64/fpu/fsetexcptflg.c
@@ -22,30 +22,34 @@
int
fesetexceptflag (const fexcept_t *flagp, int excepts)
{
+ /* The flags can be set in the 387 unit or in the SSE unit.
+ When we need to clear a flag, we need to do so in both units,
+ due to the way fetestexcept() is implemented.
+ When we need to set a flag, it is sufficient to do it in the SSE unit,
+ because that is guaranteed to not trap. */
+
fenv_t temp;
unsigned int mxcsr;
- /* XXX: Do we really need to set both the exception in both units?
- Shouldn't it be enough to set only the SSE unit? */
+ excepts &= FE_ALL_EXCEPT;
/* Get the current x87 FPU environment. We have to do this since we
cannot separately set the status word. */
__asm__ ("fnstenv %0" : "=m" (*&temp));
- temp.__status_word &= ~(excepts & FE_ALL_EXCEPT);
- temp.__status_word |= *flagp & excepts & FE_ALL_EXCEPT;
+ /* Clear relevant flags. */
+ temp.__status_word &= ~(excepts & ~ *flagp);
- /* Store the new status word (along with the rest of the environment.
- Possibly new exceptions are set but they won't get executed unless
- the next floating-point instruction. */
+ /* Store the new status word (along with the rest of the environment). */
__asm__ ("fldenv %0" : : "m" (*&temp));
- /* And now the same for SSE. */
+ /* And now similarly for SSE. */
__asm__ ("stmxcsr %0" : "=m" (*&mxcsr));
- mxcsr &= ~(excepts & FE_ALL_EXCEPT);
- mxcsr |= *flagp & excepts & FE_ALL_EXCEPT;
+ /* Clear or set relevant flags. */
+ mxcsr ^= (mxcsr ^ *flagp) & excepts;
+ /* Put the new data in effect. */
__asm__ ("ldmxcsr %0" : : "m" (*&mxcsr));
/* Success. */
--
2.34.1
next prev parent reply other threads:[~2023-10-23 21:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-23 21:36 [PATCH 0/3] Fix fesetexcept/fesetexceptflag on powerpc and x86 Adhemerval Zanella
2023-10-23 21:36 ` [PATCH 1/3] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988) Adhemerval Zanella
2023-10-23 23:50 ` Bruno Haible
2023-10-23 21:36 ` [PATCH 2/3] i686: Do not raise exception traps on fesetexcept (BZ 30989) Adhemerval Zanella
2023-10-23 23:32 ` Joseph Myers
2023-10-24 0:17 ` Bruno Haible
2023-10-24 11:12 ` Adhemerval Zanella Netto
2023-10-24 13:35 ` Bruno Haible
2023-10-23 21:36 ` Adhemerval Zanella [this message]
2023-10-24 0:19 ` [PATCH 3/3] x86: Do not raises floating-point exception traps on fesetexceptflag (BZ 30990) Bruno Haible
2023-10-24 11:13 ` Adhemerval Zanella Netto
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=20231023213659.3236496-4-adhemerval.zanella@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=bruno@clisp.org \
--cc=libc-alpha@sourceware.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).