[PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[joel]

From: Joel Sherrill <joel@rtems.org>

---
 newlib/libc/machine/i386/sys/fenv.h    |   1 +
 newlib/libc/machine/x86_64/sys/fenv.h  | 170 ++++++++++++
 newlib/libm/machine/i386/Makefile.am   |   2 +-
 newlib/libm/machine/i386/fenv.c        |   1 +
 newlib/libm/machine/x86_64/Makefile.am |  18 ++
 newlib/libm/machine/x86_64/fenv.c      | 487 +++++++++++++++++++++++++++++++++
 6 files changed, 678 insertions(+), 1 deletion(-)
 create mode 120000 newlib/libc/machine/i386/sys/fenv.h
 create mode 100644 newlib/libc/machine/x86_64/sys/fenv.h
 create mode 120000 newlib/libm/machine/i386/fenv.c
 create mode 100644 newlib/libm/machine/x86_64/Makefile.am
 create mode 100644 newlib/libm/machine/x86_64/fenv.c

diff --git a/newlib/libc/machine/i386/sys/fenv.h b/newlib/libc/machine/i386/sys/fenv.h
new file mode 120000
index 0000000..2180578
--- /dev/null
+++ b/newlib/libc/machine/i386/sys/fenv.h
@@ -0,0 +1 @@
+../../x86_64/sys/fenv.h
\ No newline at end of file
diff --git a/newlib/libc/machine/x86_64/sys/fenv.h b/newlib/libc/machine/x86_64/sys/fenv.h
new file mode 100644
index 0000000..83f5995
--- /dev/null
+++ b/newlib/libc/machine/x86_64/sys/fenv.h
@@ -0,0 +1,170 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2010-2019 Red Hat, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef _SYS_FENV_H
+#define _SYS_FENV_H 1
+
+#include <sys/cdefs.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+
+/* Primary sources:
+
+     The Open Group Base Specifications Issue 6:
+   http://www.opengroup.org/onlinepubs/000095399/basedefs/fenv.h.html
+
+     C99 Language spec (draft n1256):
+   <url unknown>
+
+     Intel(R) 64 and IA-32 Architectures Software Developer's Manuals:
+   http://www.intel.com/products/processor/manuals/
+
+     GNU C library manual pages:
+   http://www.gnu.org/software/libc/manual/html_node/Control-Functions.html
+   http://www.gnu.org/software/libc/manual/html_node/Rounding.html
+   http://www.gnu.org/software/libc/manual/html_node/FP-Exceptions.html
+   http://www.gnu.org/software/libc/manual/html_node/Status-bit-operations.html
+
+     Linux online man page(s):
+   http://linux.die.net/man/3/fegetexcept
+
+    The documentation quotes these sources for reference.  All definitions and
+   code have been developed solely based on the information from these specs.
+
+*/
+
+/*  Represents the entire floating-point environment. The floating-point
+   environment refers collectively to any floating-point status flags and
+   control modes supported by the implementation.
+    In this implementation, the struct contains the state information from
+   the fstenv/fnstenv instructions and a copy of the SSE MXCSR, since GCC
+   uses SSE for a lot of floating-point operations.  (Cygwin assumes i686
+   or above these days, as does the compiler.)  */
+
+typedef struct _fenv_t
+{
+  struct _fpu_env_info {
+    unsigned int _fpu_cw;	/* low 16 bits only. */
+    unsigned int _fpu_sw;	/* low 16 bits only. */
+    unsigned int _fpu_tagw;	/* low 16 bits only. */
+    unsigned int _fpu_ipoff;
+    unsigned int _fpu_ipsel;
+    unsigned int _fpu_opoff;
+    unsigned int _fpu_opsel;	/* low 16 bits only. */
+  } _fpu;
+  unsigned int _sse_mxcsr;
+} fenv_t;
+
+/*  Represents the floating-point status flags collectively, including
+   any status the implementation associates with the flags. A floating-point
+   status flag is a system variable whose value is set (but never cleared)
+   when a floating-point exception is raised, which occurs as a side effect
+   of exceptional floating-point arithmetic to provide auxiliary information.
+    A floating-point control mode is a system variable whose value may be
+   set by the user to affect the subsequent behavior of floating-point
+   arithmetic. */
+
+typedef __uint32_t fexcept_t;
+
+/*  The <fenv.h> header shall define the following constants if and only
+   if the implementation supports the floating-point exception by means
+   of the floating-point functions feclearexcept(), fegetexceptflag(),
+   feraiseexcept(), fesetexceptflag(), and fetestexcept(). Each expands to
+   an integer constant expression with values such that bitwise-inclusive
+   ORs of all combinations of the constants result in distinct values.  */
+
+#define FE_DIVBYZERO	(1 << 2)
+#define FE_INEXACT	(1 << 5)
+#define FE_INVALID	(1 << 0)
+#define FE_OVERFLOW	(1 << 3)
+#define FE_UNDERFLOW	(1 << 4)
+
+/*  The <fenv.h> header shall define the following constant, which is
+   simply the bitwise-inclusive OR of all floating-point exception
+   constants defined above:  */
+
+/* in agreement w/ Linux the subnormal exception will always be masked */
+#define FE_ALL_EXCEPT \
+  (FE_INEXACT | FE_UNDERFLOW | FE_OVERFLOW | FE_DIVBYZERO | FE_INVALID)
+
+/*  The <fenv.h> header shall define the following constants if and only
+   if the implementation supports getting and setting the represented
+   rounding direction by means of the fegetround() and fesetround()
+   functions. Each expands to an integer constant expression whose values
+   are distinct non-negative vales.  */
+
+#define FE_DOWNWARD	(1)
+#define FE_TONEAREST	(0)
+#define FE_TOWARDZERO	(3)
+#define FE_UPWARD	(2)
+
+/* Only Solaris and QNX implement fegetprec/fesetprec.  As Solaris, use the
+   values defined by http://www.open-std.org/jtc1/sc22//WG14/www/docs/n752.htm
+   QNX defines different values. */
+#if __MISC_VISIBLE
+#define FE_FLTPREC	(0)
+#define FE_DBLPREC	(2)
+#define FE_LDBLPREC	(3)
+#endif
+
+/*  The <fenv.h> header shall define the following constant, which
+   represents the default floating-point environment (that is, the one
+   installed at program startup) and has type pointer to const-qualified
+   fenv_t. It can be used as an argument to the functions within the
+   <fenv.h> header that manage the floating-point environment.  */
+
+extern const fenv_t *_fe_dfl_env;
+#define FE_DFL_ENV (_fe_dfl_env)
+
+/*  Additional implementation-defined environments, with macro
+   definitions beginning with FE_ and an uppercase letter,and having
+   type "pointer to const-qualified fenv_t",may also be specified by
+   the implementation.  */
+
+#if __GNU_VISIBLE
+/*  If possible, the GNU C Library defines a macro FE_NOMASK_ENV which
+   represents an environment where every exception raised causes a trap
+   to occur. You can test for this macro using #ifdef. It is only defined
+   if _GNU_SOURCE is defined.  */
+extern const fenv_t *_fe_nomask_env;
+#define FE_NOMASK_ENV (_fe_nomask_env)
+#endif /* __GNU_VISIBLE */
+
+#ifdef __INSIDE_CYGWIN__
+/* This is Cygwin-custom, not from the standard, for use in the Cygwin CRT.  */
+extern void _feinitialise ();
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _FENV_H */
diff --git a/newlib/libm/machine/i386/Makefile.am b/newlib/libm/machine/i386/Makefile.am
index 6fade2d..8880ff1 100644
--- a/newlib/libm/machine/i386/Makefile.am
+++ b/newlib/libm/machine/i386/Makefile.am
@@ -12,7 +12,7 @@ LIB_SOURCES = \
 	f_log.S f_logf.S f_log10.S f_log10f.S \
 	f_ldexp.S f_ldexpf.S f_lrint.c f_lrintf.c f_lrintl.c \
 	f_pow.c f_powf.c f_rint.c f_rintf.c f_rintl.c \
-	f_tan.S f_tanf.S f_math.h \
+	f_tan.S f_tanf.S f_math.h fenv.c \
 	i386mach.h 
 
 libi386_la_LDFLAGS = -Xcompiler -nostdlib
diff --git a/newlib/libm/machine/i386/fenv.c b/newlib/libm/machine/i386/fenv.c
new file mode 120000
index 0000000..1d7c7a1
--- /dev/null
+++ b/newlib/libm/machine/i386/fenv.c
@@ -0,0 +1 @@
+../x86_64/fenv.c
\ No newline at end of file
diff --git a/newlib/libm/machine/x86_64/Makefile.am b/newlib/libm/machine/x86_64/Makefile.am
new file mode 100644
index 0000000..f33100f
--- /dev/null
+++ b/newlib/libm/machine/x86_64/Makefile.am
@@ -0,0 +1,18 @@
+## Process this file with automake to generate Makefile.in
+
+INCLUDES = -I $(newlib_basedir)/../newlib/libm/common $(NEWLIB_CFLAGS) \
+	$(CROSS_CFLAGS) $(TARGET_CFLAGS)
+
+LIB_SOURCES = \
+	fenv.c
+
+noinst_LIBRARIES = lib.a
+lib_a_SOURCES = $(LIB_SOURCES)
+lib_a_CFLAGS = $(AM_CFLAGS)
+lib_a_CCASFLAGS = $(AM_CCASFLAGS)
+noinst_DATA =
+
+include $(srcdir)/../../../Makefile.shared
+
+ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
+CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
diff --git a/newlib/libm/machine/x86_64/fenv.c b/newlib/libm/machine/x86_64/fenv.c
new file mode 100644
index 0000000..7f6ebd4
--- /dev/null
+++ b/newlib/libm/machine/x86_64/fenv.c
@@ -0,0 +1,487 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2010-2019 Red Hat, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <fenv.h>
+#include <errno.h>
+#include <string.h>        // for memcpy 
+#include <stdbool.h>
+
+/*  x87 supports subnormal numbers so we need it below. */
+#define __FE_DENORM	(1 << 1)
+/* mask (= 0x3f) to disable all exceptions at initialization */
+#define __FE_ALL_EXCEPT_X86 (FE_ALL_EXCEPT | __FE_DENORM)
+
+/*  Mask and shift amount for rounding bits.  */
+#define FE_CW_ROUND_MASK	(0x0c00)
+#define FE_CW_ROUND_SHIFT	(10)
+/*  Same, for SSE MXCSR.  */
+#define FE_MXCSR_ROUND_MASK	(0x6000)
+#define FE_MXCSR_ROUND_SHIFT	(13)
+
+/*  Mask and shift amount for precision bits.  */
+#define FE_CW_PREC_MASK		(0x0300)
+#define FE_CW_PREC_SHIFT	(8)
+
+/*  In x87, exception status bits and mask bits occupy
+   corresponding bit positions in the status and control
+   registers, respectively.  In SSE, they are both located
+   in the control-and-status register, with the status bits
+   corresponding to the x87 positions, and the mask bits
+   shifted by this amount to the left.  */
+#define FE_SSE_EXCEPT_MASK_SHIFT (7)
+
+/* These are writable so we can initialise them at startup.  */
+static fenv_t fe_nomask_env;
+
+/* These pointers provide the outside world with read-only access to them.  */
+const fenv_t *_fe_nomask_env = &fe_nomask_env;
+
+/*  Although Cygwin assumes i686 or above (hence SSE available) these
+   days, and the compiler feels free to use it (depending on compile-
+   time flags of course), we should avoid needlessly breaking any
+   purely integer mode apps (or apps compiled with -mno-sse), so we
+   only manage SSE state in this fenv module if we detect that SSE
+   instructions are available at runtime.  If we didn't do this, all
+   applications run on older machines would bomb out with an invalid
+   instruction exception right at startup; let's not be *that* WJM!  */
+static bool use_sse = false;
+
+/*  This function enables traps for each of the exceptions as indicated
+   by the parameter except. The individual exceptions are described in
+   [ ... glibc manual xref elided ...]. Only the specified exceptions are
+   enabled, the status of the other exceptions is not changed.
+    The function returns the previous enabled exceptions in case the
+   operation was successful, -1 otherwise.  */
+int
+feenableexcept (int excepts)
+{
+  unsigned short cw, old_cw;
+  unsigned int mxcsr = 0;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return -1;
+
+  /* Get control words.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (old_cw) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (mxcsr) : );
+
+  /* Enable exceptions by clearing mask bits.  */
+  cw = old_cw & ~excepts;
+  mxcsr &= ~(excepts << FE_SSE_EXCEPT_MASK_SHIFT);
+
+  /* Store updated control words.  */
+  __asm__ volatile ("fldcw %0" :: "m" (cw));
+  if (use_sse)
+    __asm__ volatile ("ldmxcsr %0" :: "m" (mxcsr));
+
+  /* Return old value.  We assume SSE and x87 stay in sync.  Note that
+     we are returning a mask of enabled exceptions, which is the opposite
+     of the flags in the register, which are set to disable (mask) their
+     related exceptions.  */
+  return (~old_cw) & FE_ALL_EXCEPT;
+}
+
+/*  This function disables traps for each of the exceptions as indicated
+   by the parameter except. The individual exceptions are described in
+   [ ... glibc manual xref elided ...]. Only the specified exceptions are
+   disabled, the status of the other exceptions is not changed.
+    The function returns the previous enabled exceptions in case the
+   operation was successful, -1 otherwise.  */
+int
+fedisableexcept (int excepts)
+{
+  unsigned short cw, old_cw;
+  unsigned int mxcsr = 0;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return -1;
+
+  /* Get control words.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (old_cw) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (mxcsr) : );
+
+  /* Disable exceptions by setting mask bits.  */
+  cw = old_cw | excepts;
+  mxcsr |= (excepts << FE_SSE_EXCEPT_MASK_SHIFT);
+
+  /* Store updated control words.  */
+  __asm__ volatile ("fldcw %0" :: "m" (cw));
+  if (use_sse)
+    __asm__ volatile ("ldmxcsr %0" :: "m" (mxcsr));
+
+  /* Return old value.  We assume SSE and x87 stay in sync.  Note that
+     we are returning a mask of enabled exceptions, which is the opposite
+     of the flags in the register, which are set to disable (mask) their
+     related exceptions.  */
+  return (~old_cw) & FE_ALL_EXCEPT;
+}
+
+/*  This function returns a bitmask of all currently enabled exceptions. It
+   returns -1 in case of failure.  */
+int
+fegetexcept (void)
+{
+  unsigned short cw;
+
+  /* Get control word.  We assume SSE and x87 stay in sync.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (cw) : );
+
+  /* Exception is *dis*abled when mask bit is set.  */
+  return (~cw) & FE_ALL_EXCEPT;
+}
+
+/*  Store the floating-point environment in the variable pointed to by envp.
+   The function returns zero in case the operation was successful, a non-zero
+   value otherwise.  */
+int
+fegetenv (fenv_t *envp)
+{
+  /* fnstenv disables all exceptions in the x87 FPU; as this is not what is
+     desired here, reload the cfg saved from the x87 FPU, back to the FPU */
+  __asm__ volatile ("fnstenv %0\n\
+                     fldenv %0"
+		    : "=m" (envp->_fpu) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (envp->_sse_mxcsr) : );
+  return 0;
+}
+
+/*  Store the current floating-point environment in the object pointed to
+   by envp. Then clear all exception flags, and set the FPU to trap no
+   exceptions.  Not all FPUs support trapping no exceptions; if feholdexcept
+   cannot set this mode, it returns nonzero value.  If it succeeds, it
+   returns zero.  */
+int
+feholdexcept (fenv_t *envp)
+{
+  unsigned int mxcsr;
+  fegetenv (envp);
+  mxcsr = envp->_sse_mxcsr & ~FE_ALL_EXCEPT;
+  if (use_sse)
+    __asm__ volatile ("ldmxcsr %0" :: "m" (mxcsr));
+  __asm__ volatile ("fnclex");
+  fedisableexcept (FE_ALL_EXCEPT);
+  return 0;
+}
+
+/*  Set the floating-point environment to that described by envp.  The
+   function returns zero in case the operation was successful, a non-zero
+   value otherwise.  */
+int
+fesetenv (const fenv_t *envp)
+{
+  __asm__ volatile ("fldenv %0" :: "m" (envp->_fpu) );
+  if (use_sse)
+    __asm__ volatile ("ldmxcsr %0" :: "m" (envp->_sse_mxcsr));
+  return 0;
+}
+
+/*  Like fesetenv, this function sets the floating-point environment to
+   that described by envp. However, if any exceptions were flagged in the
+   status word before feupdateenv was called, they remain flagged after
+   the call.  In other words, after feupdateenv is called, the status
+   word is the bitwise OR of the previous status word and the one saved
+   in envp.  The function returns zero in case the operation was successful,
+   a non-zero value otherwise.  */
+int
+feupdateenv (const fenv_t *envp)
+{
+  fenv_t envcopy;
+  unsigned int mxcsr = 0;
+  unsigned short sw;
+
+  /* Don't want to modify *envp, but want to update environment atomically,
+     so take a copy and merge the existing exceptions into it.  */
+  memcpy (&envcopy, envp, sizeof *envp);
+  __asm__ volatile ("fnstsw %0" : "=m" (sw) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (mxcsr) : );
+  envcopy._fpu._fpu_sw |= (sw & FE_ALL_EXCEPT);
+  envcopy._sse_mxcsr |= (mxcsr & FE_ALL_EXCEPT);
+
+  return fesetenv (&envcopy);
+}
+
+/*  This function clears all of the supported exception flags indicated by
+   excepts.  The function returns zero in case the operation was successful,
+   a non-zero value otherwise.  */
+int
+feclearexcept (int excepts)
+{
+  fenv_t fenv;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return EINVAL;
+
+  /* Need to save/restore whole environment to modify status word.  */
+  fegetenv (&fenv);
+
+  /* Mask undesired bits out.  */
+  fenv._fpu._fpu_sw &= ~excepts;
+  fenv._sse_mxcsr &= ~excepts;
+
+  /* Set back into FPU state.  */
+  return fesetenv (&fenv);
+}
+
+/*  This function raises the supported exceptions indicated by
+   excepts.  If more than one exception bit in excepts is set the order
+   in which the exceptions are raised is undefined except that overflow
+   (FE_OVERFLOW) or underflow (FE_UNDERFLOW) are raised before inexact
+   (FE_INEXACT). Whether for overflow or underflow the inexact exception
+   is also raised is also implementation dependent.  The function returns
+   zero in case the operation was successful, a non-zero value otherwise.  */
+int
+feraiseexcept (int excepts)
+{
+  fenv_t fenv;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return EINVAL;
+
+  /* Need to save/restore whole environment to modify status word.  */
+  __asm__ volatile ("fnstenv %0" : "=m" (fenv) : );
+
+  /* Set desired exception bits.  */
+  fenv._fpu._fpu_sw |= excepts;
+
+  /* Set back into FPU state.  */
+  __asm__ volatile ("fldenv %0" :: "m" (fenv));
+
+  /* And trigger them - whichever are unmasked.  */
+  __asm__ volatile ("fwait");
+
+  return 0;
+}
+
+/*  Test whether the exception flags indicated by the parameter except
+   are currently set. If any of them are, a nonzero value is returned
+   which specifies which exceptions are set. Otherwise the result is zero.  */
+int
+fetestexcept (int excepts)
+{
+  unsigned short sw;
+  unsigned int mxcsr = 0;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return EINVAL;
+
+  /* Get status registers.  */
+  __asm__ volatile ("fnstsw %0" : "=m" (sw) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (mxcsr) : );
+
+  /* Mask undesired bits out and return result.  */
+  return (sw | mxcsr) & excepts;
+}
+/*  This function stores in the variable pointed to by flagp an
+   implementation-defined value representing the current setting of the
+   exception flags indicated by excepts.  The function returns zero in
+   case the operation was successful, a non-zero value otherwise.  */
+int
+fegetexceptflag (fexcept_t *flagp, int excepts)
+{
+  unsigned short sw;
+  unsigned int mxcsr = 0;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return EINVAL;
+
+  /* Get status registers.  */
+  __asm__ volatile ("fnstsw %0" : "=m" (sw) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (mxcsr) : );
+
+  /* Mask undesired bits out and set result.  */
+  *flagp = (sw | mxcsr) & excepts;
+
+  return 0;
+}
+
+/*  This function restores the flags for the exceptions indicated by
+   excepts to the values stored in the variable pointed to by flagp.  */
+int
+fesetexceptflag (const fexcept_t *flagp, int excepts)
+{
+  fenv_t fenv;
+
+  if (excepts & ~FE_ALL_EXCEPT)
+    return EINVAL;
+
+  /* Need to save/restore whole environment to modify status word.  */
+  fegetenv (&fenv);
+
+  /* Set/Clear desired exception bits.  */
+  fenv._fpu._fpu_sw &= ~excepts;
+  fenv._fpu._fpu_sw |= excepts & *flagp;
+  fenv._sse_mxcsr &= ~excepts;
+  fenv._sse_mxcsr |= excepts & *flagp;
+
+  /* Set back into FPU state.  */
+  return fesetenv (&fenv);
+}
+
+/*  Returns the currently selected rounding mode, represented by one of the
+   values of the defined rounding mode macros.  */
+int
+fegetround (void)
+{
+  unsigned short cw;
+
+  /* Get control word.  We assume SSE and x87 stay in sync.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (cw) : );
+
+  return (cw & FE_CW_ROUND_MASK) >> FE_CW_ROUND_SHIFT;
+}
+
+/*  Changes the currently selected rounding mode to round. If round does
+   not correspond to one of the supported rounding modes nothing is changed.
+   fesetround returns zero if it changed the rounding mode, a nonzero value
+   if the mode is not supported.  */
+int
+fesetround (int round)
+{
+  unsigned short cw;
+  unsigned int mxcsr = 0;
+
+  /* Will succeed for any valid value of the input parameter.  */
+  if (round < FE_TONEAREST || round > FE_TOWARDZERO)
+    return EINVAL;
+
+  /* Get control words.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (cw) : );
+  if (use_sse)
+    __asm__ volatile ("stmxcsr %0" : "=m" (mxcsr) : );
+
+  /* Twiddle bits.  */
+  cw &= ~FE_CW_ROUND_MASK;
+  cw |= (round << FE_CW_ROUND_SHIFT);
+  mxcsr &= ~FE_MXCSR_ROUND_MASK;
+  mxcsr |= (round << FE_MXCSR_ROUND_SHIFT);
+
+  /* Set back into FPU state.  */
+  __asm__ volatile ("fldcw %0" :: "m" (cw));
+  if (use_sse)
+    __asm__ volatile ("ldmxcsr %0" :: "m" (mxcsr));
+
+  /* Indicate success.  */
+  return 0;
+}
+
+#if defined(__CYGWIN__)
+/*  Returns the currently selected precision, represented by one of the
+   values of the defined precision macros.  */
+int
+fegetprec (void)
+{
+  unsigned short cw;
+
+  /* Get control word.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (cw) : );
+
+  return (cw & FE_CW_PREC_MASK) >> FE_CW_PREC_SHIFT;
+}
+
+/* http://www.open-std.org/jtc1/sc22//WG14/www/docs/n752.htm:
+
+   The fesetprec function establishes the precision represented by its
+   argument prec.  If the argument does not match a precision macro, the
+   precision is not changed.
+
+   The fesetprec function returns a nonzero value if and only if the
+   argument matches a precision macro (that is, if and only if the requested
+   precision can be established). */
+int
+fesetprec (int prec)
+{
+  unsigned short cw;
+
+  /* Will succeed for any valid value of the input parameter.  */
+  switch (prec)
+    {
+    case FE_FLTPREC:
+    case FE_DBLPREC:
+    case FE_LDBLPREC:
+      break;
+    default:
+      return 0;
+    }
+
+  /* Get control word.  */
+  __asm__ volatile ("fnstcw %0" : "=m" (cw) : );
+
+  /* Twiddle bits.  */
+  cw &= ~FE_CW_PREC_MASK;
+  cw |= (prec << FE_CW_PREC_SHIFT);
+
+  /* Set back into FPU state.  */
+  __asm__ volatile ("fldcw %0" :: "m" (cw));
+
+  /* Indicate success.  */
+  return 1;
+}
+#endif
+
+/*  Set up the FPU and SSE environment at the start of execution.  */
+void
+_feinitialise (void)
+{
+  unsigned int edx, eax;
+  extern fenv_t __fe_dfl_env;
+
+  /* Check for presence of SSE: invoke CPUID #1, check EDX bit 25.  */
+  eax = 1;
+  __asm__ volatile ("cpuid" : "=d" (edx), "+a" (eax) :: "%ecx", "%ebx");
+  /* If this flag isn't set we'll avoid trying to execute any SSE.  */
+  if ((edx & (1 << 25)) != 0)
+    use_sse = true;
+
+  /* Reset FPU: extended prec, all exceptions cleared and masked off.  */
+  __asm__ volatile ("fninit");
+  /* The default cw value, 0x37f, is rounding mode zero.  The MXCSR has
+     no precision control, so the only thing to do is set the exception
+     mask bits.  */
+
+  /* initialize the MXCSR register: mask all exceptions */
+  unsigned int mxcsr = __FE_ALL_EXCEPT_X86 << FE_SSE_EXCEPT_MASK_SHIFT;
+  if (use_sse)
+    __asm__ volatile ("ldmxcsr %0" :: "m" (mxcsr));
+
+  /* Setup unmasked environment, but leave __FE_DENORM masked.  */
+  feenableexcept (FE_ALL_EXCEPT);
+  fegetenv (&fe_nomask_env);
+
+  /* Restore default exception masking (all masked).  */
+  fedisableexcept (FE_ALL_EXCEPT);
+
+  /* Finally cache state as default environment. */
+  fegetenv (&__fe_dfl_env);
+}
+
-- 
1.8.3.1

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 4841 bytes --]

Hi Joel,

On Sep  3 11:02, joel@rtems.org wrote:
> index 0000000..83f5995
> --- /dev/null
> +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> @@ -0,0 +1,170 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2010-2019 Red Hat, Inc.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */

The SPDX-License-Identifier usually replaces the license text...

> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2010-2019 Red Hat, Inc.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */

Ditto.

> +/* These are writable so we can initialise them at startup.  */
> +static fenv_t fe_nomask_env;
> +
> +/* These pointers provide the outside world with read-only access to them.  */
> +const fenv_t *_fe_nomask_env = &fe_nomask_env;
> +

Didn't we talk about defining those in their own file?

> +/*  Although Cygwin assumes i686 or above (hence SSE available) these

Still, please drop Cygwin-specific comments.

> +/*  Set the floating-point environment to that described by envp.  The
> +   function returns zero in case the operation was successful, a non-zero
> +   value otherwise.  */
> +int
> +fesetenv (const fenv_t *envp)
> +{
> +  __asm__ volatile ("fldenv %0" :: "m" (envp->_fpu) );
> +  if (use_sse)
> +    __asm__ volatile ("ldmxcsr %0" :: "m" (envp->_sse_mxcsr));
> +  return 0;
> +}

I'm still not happy that all targets have to call _feinitialise() at
some arbitrary point in their init code to set up FE_DFL_ENV and
FE_NOMASK_ENV.  That may work for Cygwin with the Cygwin DLL being an
integral part of all processes, but for other targets that only makes
marginal sense, especially if no fenv functions are used.

So, here's an idea:

int
fesetenv (const fenv_t *envp)
{
  if ((envp == FE_DFL_ENV || envp == FE_NOMASK_ENV)
      && envp->_fpu._fpu_cw == 0)
    _feinitialise ();
  [...]
}

In theory, this single test should cover all bases.  Even Cygwin
can drop the _feinitialise() call from its init code then.

Am I missing something?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Joel Sherrill]

On Wed, Sep 4, 2019 at 3:19 AM Corinna Vinschen <vinschen@redhat.com> wrote:
>
> Hi Joel,
>
> On Sep  3 11:02, joel@rtems.org wrote:
> > index 0000000..83f5995
> > --- /dev/null
> > +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> > @@ -0,0 +1,170 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2010-2019 Red Hat, Inc.
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE.
> > + */
>
> The SPDX-License-Identifier usually replaces the license text...

Maybe in principle but I couldn't find a single file in newlib which had only
an SPDX indicator. All 63 had SPDX and license text.

I don't see changing the pattern for this file and I doubt we will be deleting
copyright/license text from at least FreeBSD sourced code since that will
make it harder to track against the upstream.

>
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2010-2019 Red Hat, Inc.
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE.
> > + */
>
> Ditto.
>
> > +/* These are writable so we can initialise them at startup.  */
> > +static fenv_t fe_nomask_env;
> > +
> > +/* These pointers provide the outside world with read-only access to them.  */
> > +const fenv_t *_fe_nomask_env = &fe_nomask_env;
> > +
>
> Didn't we talk about defining those in their own file?

I had a patch which added them to the stub support but they aren't
generic C/POSIX methods and there was a comment that they
should be target specific.

By keeping them in the machine sys/fenv.h and defining them
in the machine specific fenv.c, I thought that addressed that.

> > +/*  Although Cygwin assumes i686 or above (hence SSE available) these
>
> Still, please drop Cygwin-specific comments.

I forgot to do that. First sentence rewritten.

> > +/*  Set the floating-point environment to that described by envp.  The
> > +   function returns zero in case the operation was successful, a non-zero
> > +   value otherwise.  */
> > +int
> > +fesetenv (const fenv_t *envp)
> > +{
> > +  __asm__ volatile ("fldenv %0" :: "m" (envp->_fpu) );
> > +  if (use_sse)
> > +    __asm__ volatile ("ldmxcsr %0" :: "m" (envp->_sse_mxcsr));
> > +  return 0;
> > +}
>
> I'm still not happy that all targets have to call _feinitialise() at
> some arbitrary point in their init code to set up FE_DFL_ENV and
> FE_NOMASK_ENV.  That may work for Cygwin with the Cygwin DLL being an
> integral part of all processes, but for other targets that only makes
> marginal sense, especially if no fenv functions are used.
>
> So, here's an idea:
>
> int
> fesetenv (const fenv_t *envp)
> {
>   if ((envp == FE_DFL_ENV || envp == FE_NOMASK_ENV)
>       && envp->_fpu._fpu_cw == 0)
>     _feinitialise ();
>   [...]
> }
>
> In theory, this single test should cover all bases.  Even Cygwin
> can drop the _feinitialise() call from its init code then.
>
> Am I missing something?

I think _feinitialize can now be static. I moved it to the top of the file

Doing that lets the prototype and a CYGWIN specific comment in the sys/fenv.h
disappear.

Does that sound ok?

--joel

>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]

On Sep  4 07:43, Joel Sherrill wrote:
> On Wed, Sep 4, 2019 at 3:19 AM Corinna Vinschen <vinschen@redhat.com> wrote:
> > On Sep  3 11:02, joel@rtems.org wrote:
> > > index 0000000..83f5995
> > > --- /dev/null
> > > +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> > > @@ -0,0 +1,170 @@
> > > +/*
> > > + * SPDX-License-Identifier: BSD-2-Clause
> > > + *
> > > + * Copyright (c) 2010-2019 Red Hat, Inc.
> > > + * All rights reserved.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or without
> > > + * modification, are permitted provided that the following conditions
> > > + * are met:
> > > + * 1. Redistributions of source code must retain the above copyright
> > > + *    notice, this list of conditions and the following disclaimer.
> > > + * 2. Redistributions in binary form must reproduce the above copyright
> > > + *    notice, this list of conditions and the following disclaimer in the
> > > + *    documentation and/or other materials provided with the distribution.
> > > + *
> > > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > > + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > > + * SUCH DAMAGE.
> > > + */
> >
> > The SPDX-License-Identifier usually replaces the license text...
> 
> Maybe in principle but I couldn't find a single file in newlib which had only
> an SPDX indicator. All 63 had SPDX and license text.

I didn't actually check newlib files, but SPDX + written out license
only makes marginal sense.  So, let's start with *these* files to do it
right, please.

> > Didn't we talk about defining those in their own file?
> 
> I had a patch which added them to the stub support but they aren't
> generic C/POSIX methods and there was a comment that they
> should be target specific.

Hmm, yeah, right.  I missed that, sorry.

> > I'm still not happy that all targets have to call _feinitialise() at
> > some arbitrary point in their init code to set up FE_DFL_ENV and
> > FE_NOMASK_ENV.  That may work for Cygwin with the Cygwin DLL being an
> > integral part of all processes, but for other targets that only makes
> > marginal sense, especially if no fenv functions are used.
> >
> > So, here's an idea:
> >
> > int
> > fesetenv (const fenv_t *envp)
> > {
> >   if ((envp == FE_DFL_ENV || envp == FE_NOMASK_ENV)
> >       && envp->_fpu._fpu_cw == 0)
> >     _feinitialise ();
> >   [...]
> > }
> >
> > In theory, this single test should cover all bases.  Even Cygwin
> > can drop the _feinitialise() call from its init code then.
> >
> > Am I missing something?
> 
> I think _feinitialize can now be static. I moved it to the top of the file
> 
> Doing that lets the prototype and a CYGWIN specific comment in the sys/fenv.h
> disappear.
> 
> Does that sound ok?

That sounds right to me.  Let's do that for now.

With the above changes I can push the patch, but I won't probably be
able to test this on Cygwin until after my vacation.  It might be
better to go ahead with the current code in Cygwin for the Cygwin 3.1
release and only afterr that I'll start switching, ok?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Joel Sherrill]

On Wed, Sep 4, 2019 at 10:12 AM Corinna Vinschen <vinschen@redhat.com> wrote:
>
> On Sep  4 07:43, Joel Sherrill wrote:
> > On Wed, Sep 4, 2019 at 3:19 AM Corinna Vinschen <vinschen@redhat.com> wrote:
> > > On Sep  3 11:02, joel@rtems.org wrote:
> > > > index 0000000..83f5995
> > > > --- /dev/null
> > > > +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> > > > @@ -0,0 +1,170 @@
> > > > +/*
> > > > + * SPDX-License-Identifier: BSD-2-Clause
> > > > + *
> > > > + * Copyright (c) 2010-2019 Red Hat, Inc.
> > > > + * All rights reserved.
> > > > + *
> > > > + * Redistribution and use in source and binary forms, with or without
> > > > + * modification, are permitted provided that the following conditions
> > > > + * are met:
> > > > + * 1. Redistributions of source code must retain the above copyright
> > > > + *    notice, this list of conditions and the following disclaimer.
> > > > + * 2. Redistributions in binary form must reproduce the above copyright
> > > > + *    notice, this list of conditions and the following disclaimer in the
> > > > + *    documentation and/or other materials provided with the distribution.
> > > > + *
> > > > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > > > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > > > + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > > > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > > > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > > > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > > > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > > > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > > > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > > > + * SUCH DAMAGE.
> > > > + */
> > >
> > > The SPDX-License-Identifier usually replaces the license text...
> >
> > Maybe in principle but I couldn't find a single file in newlib which had only
> > an SPDX indicator. All 63 had SPDX and license text.
>
> I didn't actually check newlib files, but SPDX + written out license
> only makes marginal sense.  So, let's start with *these* files to do it
> right, please.

SPDX line plus an organization copyright? Like this:

/*
 * SPDX-License-Identifier: BSD-2-Clause
 *
 * Copyright (c) 2010-2019 Red Hat, Inc.
 * All rights reserved.
 */

>
> > > Didn't we talk about defining those in their own file?
> >
> > I had a patch which added them to the stub support but they aren't
> > generic C/POSIX methods and there was a comment that they
> > should be target specific.
>
> Hmm, yeah, right.  I missed that, sorry.
>
> > > I'm still not happy that all targets have to call _feinitialise() at
> > > some arbitrary point in their init code to set up FE_DFL_ENV and
> > > FE_NOMASK_ENV.  That may work for Cygwin with the Cygwin DLL being an
> > > integral part of all processes, but for other targets that only makes
> > > marginal sense, especially if no fenv functions are used.
> > >
> > > So, here's an idea:
> > >
> > > int
> > > fesetenv (const fenv_t *envp)
> > > {
> > >   if ((envp == FE_DFL_ENV || envp == FE_NOMASK_ENV)
> > >       && envp->_fpu._fpu_cw == 0)
> > >     _feinitialise ();
> > >   [...]
> > > }
> > >
> > > In theory, this single test should cover all bases.  Even Cygwin
> > > can drop the _feinitialise() call from its init code then.
> > >
> > > Am I missing something?
> >
> > I think _feinitialize can now be static. I moved it to the top of the file
> >
> > Doing that lets the prototype and a CYGWIN specific comment in the sys/fenv.h
> > disappear.
> >
> > Does that sound ok?
>
> That sounds right to me.  Let's do that for now.
>
> With the above changes I can push the patch, but I won't probably be
> able to test this on Cygwin until after my vacation.  It might be
> better to go ahead with the current code in Cygwin for the Cygwin 3.1
> release and only afterr that I'll start switching, ok?

I think that's prudent for Cygwin. It had a working implementation. I don't
like to touch production code right on a Friday afternoon or before a
vacation. :)

I am testing a new version of the patch now. I can make the SPDX changes
if you confirm.

>
>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 2731 bytes --]

On 9/4/19 11:58 AM, Joel Sherrill wrote:

>>>>> +++ b/newlib/libc/machine/x86_64/sys/fenv.h
>>>>> @@ -0,0 +1,170 @@
>>>>> +/*
>>>>> + * SPDX-License-Identifier: BSD-2-Clause
>>>>> + *
>>>>> + * Copyright (c) 2010-2019 Red Hat, Inc.
>>>>> + * All rights reserved.
>>>>> + *
>>>>> + * Redistribution and use in source and binary forms, with or without
>>>>> + * modification, are permitted provided that the following conditions
>>>>> + * are met:
>>>>> + * 1. Redistributions of source code must retain the above copyright
>>>>> + *    notice, this list of conditions and the following disclaimer.
>>>>> + * 2. Redistributions in binary form must reproduce the above copyright
>>>>> + *    notice, this list of conditions and the following disclaimer in the
>>>>> + *    documentation and/or other materials provided with the distribution.
>>>>> + *
>>>>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
>>>>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>>>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>>>>> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
>>>>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>>>>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>>>>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>>>>> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT

>> I didn't actually check newlib files, but SPDX + written out license
>> only makes marginal sense.  So, let's start with *these* files to do it
>> right, please.
> 
> SPDX line plus an organization copyright? Like this:
> 
> /*
>  * SPDX-License-Identifier: BSD-2-Clause
>  *
>  * Copyright (c) 2010-2019 Red Hat, Inc.
>  * All rights reserved.
>  */
> 

It sounds funny to claim a license that grants rights to users, but yet
states "All rights reserved" that sounds like you are NOT granting
rights to others.  These days, it has lost all legal significance, and
is merely adding noise:
https://www.iusmentis.com/copyright/allrightsreserved/

With BSD license, it is a historical accident of copy-and-paste; where
different upstream sources of the license differ on whether the phrase
is still part of the license:

https://spdx.org/licenses/BSD-2-Clause.html
https://opensource.org/licenses/BSD-2-Clause

Thus, I can overlook it for BSD files, but I hate seeing it on GPL
files, so I try to point it out whenever I notice it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2880 bytes --]

On Sep  4 12:14, Eric Blake wrote:
> On 9/4/19 11:58 AM, Joel Sherrill wrote:
> 
> >>>>> +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> >>>>> @@ -0,0 +1,170 @@
> >>>>> +/*
> >>>>> + * SPDX-License-Identifier: BSD-2-Clause
> >>>>> + *
> >>>>> + * Copyright (c) 2010-2019 Red Hat, Inc.
> >>>>> + * All rights reserved.
> >>>>> + *
> >>>>> + * Redistribution and use in source and binary forms, with or without
> >>>>> + * modification, are permitted provided that the following conditions
> >>>>> + * are met:
> >>>>> + * 1. Redistributions of source code must retain the above copyright
> >>>>> + *    notice, this list of conditions and the following disclaimer.
> >>>>> + * 2. Redistributions in binary form must reproduce the above copyright
> >>>>> + *    notice, this list of conditions and the following disclaimer in the
> >>>>> + *    documentation and/or other materials provided with the distribution.
> >>>>> + *
> >>>>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> >>>>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> >>>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> >>>>> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> >>>>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> >>>>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> >>>>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> >>>>> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> 
> >> I didn't actually check newlib files, but SPDX + written out license
> >> only makes marginal sense.  So, let's start with *these* files to do it
> >> right, please.
> > 
> > SPDX line plus an organization copyright? Like this:
> > 
> > /*
> >  * SPDX-License-Identifier: BSD-2-Clause
> >  *
> >  * Copyright (c) 2010-2019 Red Hat, Inc.
> >  * All rights reserved.
> >  */
> > 
> 
> It sounds funny to claim a license that grants rights to users, but yet
> states "All rights reserved" that sounds like you are NOT granting
> rights to others.  These days, it has lost all legal significance, and
> is merely adding noise:
> https://www.iusmentis.com/copyright/allrightsreserved/
> 
> With BSD license, it is a historical accident of copy-and-paste; where
> different upstream sources of the license differ on whether the phrase
> is still part of the license:
> 
> https://spdx.org/licenses/BSD-2-Clause.html
> https://opensource.org/licenses/BSD-2-Clause
> 
> Thus, I can overlook it for BSD files, but I hate seeing it on GPL
> files, so I try to point it out whenever I notice it.

I agree, skip the reserved bit, otherwise it's fine.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Joel Sherrill]

Hi

I thought I was close to wrapping this up but I have run into a couple of issues
with this implementation being a single file and the stub being a collection
of files. After my most recent rebuild, I am getting linking conflicts because
the implementation doesn't override the stubs.

This means that each implementation needs to be multiple files with the
same names as the stub implementation. The riscv does this.but the x86
does not.

(1) Do you all agree that I need split the single fenv.c file into
multiple files?
Or should I provide empty versions of each file in libm/fenv so an
implementation
can be one file but just override the other files with common files. I
ask because
I suspect other architecture implementations will also be one file or inline the
implementation in sys/fenv.h. This would make it easy to merge those.

Suggestions?

(1a) If I do split, do I provide a file like fenvimpl.h for the constants at the
top of the file? And place it as a local file in libm/machine/i386 and x86_64?

(2) Independent of splitting the file, I noticed use_sse is used in multiple
methods but only set by _feinitialize(). I THINK the solution is to turn
use_sse into a method which dynamically checks the cpuid feature flag.
We can't rely on that always being called.

Thanks.

--joel

On Wed, Sep 4, 2019 at 1:03 PM Corinna Vinschen <vinschen@redhat.com> wrote:
>
> On Sep  4 12:14, Eric Blake wrote:
> > On 9/4/19 11:58 AM, Joel Sherrill wrote:
> >
> > >>>>> +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> > >>>>> @@ -0,0 +1,170 @@
> > >>>>> +/*
> > >>>>> + * SPDX-License-Identifier: BSD-2-Clause
> > >>>>> + *
> > >>>>> + * Copyright (c) 2010-2019 Red Hat, Inc.
> > >>>>> + * All rights reserved.
> > >>>>> + *
> > >>>>> + * Redistribution and use in source and binary forms, with or without
> > >>>>> + * modification, are permitted provided that the following conditions
> > >>>>> + * are met:
> > >>>>> + * 1. Redistributions of source code must retain the above copyright
> > >>>>> + *    notice, this list of conditions and the following disclaimer.
> > >>>>> + * 2. Redistributions in binary form must reproduce the above copyright
> > >>>>> + *    notice, this list of conditions and the following disclaimer in the
> > >>>>> + *    documentation and/or other materials provided with the distribution.
> > >>>>> + *
> > >>>>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > >>>>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > >>>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > >>>>> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > >>>>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > >>>>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > >>>>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > >>>>> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> >
> > >> I didn't actually check newlib files, but SPDX + written out license
> > >> only makes marginal sense.  So, let's start with *these* files to do it
> > >> right, please.
> > >
> > > SPDX line plus an organization copyright? Like this:
> > >
> > > /*
> > >  * SPDX-License-Identifier: BSD-2-Clause
> > >  *
> > >  * Copyright (c) 2010-2019 Red Hat, Inc.
> > >  * All rights reserved.
> > >  */
> > >
> >
> > It sounds funny to claim a license that grants rights to users, but yet
> > states "All rights reserved" that sounds like you are NOT granting
> > rights to others.  These days, it has lost all legal significance, and
> > is merely adding noise:
> > https://www.iusmentis.com/copyright/allrightsreserved/
> >
> > With BSD license, it is a historical accident of copy-and-paste; where
> > different upstream sources of the license differ on whether the phrase
> > is still part of the license:
> >
> > https://spdx.org/licenses/BSD-2-Clause.html
> > https://opensource.org/licenses/BSD-2-Clause
> >
> > Thus, I can overlook it for BSD files, but I hate seeing it on GPL
> > files, so I try to point it out whenever I notice it.
>
> I agree, skip the reserved bit, otherwise it's fine.
>
>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

On Sep  4 15:10, Joel Sherrill wrote:
> Hi
> 
> I thought I was close to wrapping this up but I have run into a couple of issues
> with this implementation being a single file and the stub being a collection
> of files. After my most recent rebuild, I am getting linking conflicts because
> the implementation doesn't override the stubs.
> 
> This means that each implementation needs to be multiple files with the
> same names as the stub implementation. The riscv does this.but the x86
> does not.
> 
> (1) Do you all agree that I need split the single fenv.c file into
> multiple files?
> Or should I provide empty versions of each file in libm/fenv so an
> implementation
> can be one file but just override the other files with common files. I
> ask because
> I suspect other architecture implementations will also be one file or inline the
> implementation in sys/fenv.h. This would make it easy to merge those.
> 
> Suggestions?

Ideally all targets define one function per file.  Cygwin is probably
the only target here which does not care for space, but it doesn't hurt
if the functions are split for Cygwin either.  So if only the actually
required functions are linked into an executable, you won't find anybody
complaining.

> (1a) If I do split, do I provide a file like fenvimpl.h for the constants at the
> top of the file? And place it as a local file in libm/machine/i386 and x86_64?

In theory this should not be necessary.  sys/fenv.h is getting overridden
by the target-specific implementation, that should be sufficient.

> (2) Independent of splitting the file, I noticed use_sse is used in multiple
> methods but only set by _feinitialize(). I THINK the solution is to turn
> use_sse into a method which dynamically checks the cpuid feature flag.
> We can't rely on that always being called.

Makes sense.  Please note that Jeff will take over from here.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Brian Inglis]

On 2019-09-03 10:02, joel@rtems.org wrote:
> From: Joel Sherrill <joel@rtems.org>

>  newlib/libc/machine/i386/sys/fenv.h    |   1 +
>  newlib/libc/machine/x86_64/sys/fenv.h  | 170 ++++++++++++

>  create mode 120000 newlib/libc/machine/i386/sys/fenv.h
>  create mode 100644 newlib/libc/machine/x86_64/sys/fenv.h

> --- /dev/null
> +++ b/newlib/libc/machine/x86_64/sys/fenv.h

> +/* Primary sources:
> +
> +     The Open Group Base Specifications Issue 6:
> +   http://www.opengroup.org/onlinepubs/000095399/basedefs/fenv.h.html
> +
> +     C99 Language spec (draft n1256):
> +   <url unknown>
      http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf (2007)
> +
> +     Intel(R) 64 and IA-32 Architectures Software Developer's Manuals:
> +   http://www.intel.com/products/processor/manuals/
> +
> +     GNU C library manual pages:
> +   http://www.gnu.org/software/libc/manual/html_node/Control-Functions.html
> +   http://www.gnu.org/software/libc/manual/html_node/Rounding.html
> +   http://www.gnu.org/software/libc/manual/html_node/FP-Exceptions.html
> +   http://www.gnu.org/software/libc/manual/html_node/Status-bit-operations.html
> +
> +     Linux online man page(s):
> +   http://linux.die.net/man/3/fegetexcept
> +
> +    The documentation quotes these sources for reference.  All definitions and
> +   code have been developed solely based on the information from these specs.
> +
> +*/

Shouldn't this header info also appear in i386/sys/fenv.h?

Draft standards are mentioned in the catalogue:

	http://www.open-std.org/jtc1/sc22/wg14/www/wg14_document_log.htm

but you have to check PDFs are unlocked - some recent drafts are locked.

Not sure why you mention C99, newer standards are available:
C2011	http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf	(2011)
C2017
https://web.archive.org/web/20181230041359if_/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf
		(2017)
C202X	http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2385.pdf	(2019)

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.

Re: [PATCH v2] Add i386 and x86_64 fenv support from Cygwin.

[Joel Sherrill]

On Fri, Sep 6, 2019, 11:58 PM Brian Inglis <Brian.Inglis@systematicsw.ab.ca>
wrote:

> On 2019-09-03 10:02, joel@rtems.org wrote:
> > From: Joel Sherrill <joel@rtems.org>
>
> >  newlib/libc/machine/i386/sys/fenv.h    |   1 +
> >  newlib/libc/machine/x86_64/sys/fenv.h  | 170 ++++++++++++
>
> >  create mode 120000 newlib/libc/machine/i386/sys/fenv.h
> >  create mode 100644 newlib/libc/machine/x86_64/sys/fenv.h
>
> > --- /dev/null
> > +++ b/newlib/libc/machine/x86_64/sys/fenv.h
>
> > +/* Primary sources:
> > +
> > +     The Open Group Base Specifications Issue 6:
> > +   http://www.opengroup.org/onlinepubs/000095399/basedefs/fenv.h.html
> > +
> > +     C99 Language spec (draft n1256):
> > +   <url unknown>
>       http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf (2007)
> > +
> > +     Intel(R) 64 and IA-32 Architectures Software Developer's Manuals:
> > +   http://www.intel.com/products/processor/manuals/
> > +
> > +     GNU C library manual pages:
> > +
> http://www.gnu.org/software/libc/manual/html_node/Control-Functions.html
> > +   http://www.gnu.org/software/libc/manual/html_node/Rounding.html
> > +   http://www.gnu.org/software/libc/manual/html_node/FP-Exceptions.html
> > +
> http://www.gnu.org/software/libc/manual/html_node/Status-bit-operations.html
> > +
> > +     Linux online man page(s):
> > +   http://linux.die.net/man/3/fegetexcept
> > +
> > +    The documentation quotes these sources for reference.  All
> definitions and
> > +   code have been developed solely based on the information from these
> specs.
> > +
> > +*/
>
> Shouldn't this header info also appear in i386/sys/fenv.h?
>

It does via symlink. All of the files should be shared.

>
> Draft standards are mentioned in the catalogue:
>
>         http://www.open-std.org/jtc1/sc22/wg14/www/wg14_document_log.htm
>
> but you have to check PDFs are unlocked - some recent drafts are locked.
>
> Not sure why you mention C99, newer standards are available:
> C2011   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
>  (2011)
> C2017
>
> https://web.archive.org/web/20181230041359if_/http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf
>                 (2017)
> C202X   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2385.pdf
>  (2019)
>

While I agree it is good to use the latest references, I didn't write this
code. I migrated it from winsup/cygwin to newlib so other environments
could use it.

IMO it would be better to migrate it and then tidy up the references. Some
shouldn't be in a machine .h in the new organization. The POSIX and C99
should be in include/fenv.h.

>
>
>
>
> --
> Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
>
> This email may be disturbing to some readers as it contains
> too much technical detail. Reader discretion is advised.
>