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

[Corinna Vinschen <vinschen at redhat dot com>]

On Aug 28 10:06, 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  | 150 ++++++++++
>  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      | 485 +++++++++++++++++++++++++++++++++
>  6 files changed, 656 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..69f7bef
> --- /dev/null
> +++ b/newlib/libc/machine/x86_64/sys/fenv.h
> @@ -0,0 +1,150 @@
> +/* fenv.h
> +
> +This file is part of Cygwin.
> +
> +This software is a copyrighted work licensed under the terms of the
> +Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
> +details. */

  SPDX-License-Identifier: BSD-2-Clause

not BSD-3-Clause as I wrote in other mail.

> +/*  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)

These can go away, right?  They are already defined in
newlib/libc/include/sys/fenv.h.

> +#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 */

And those you just added to newlib/libc/include/sys/fenv.h in patch 2 of
this set.

> +/* 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;

Given these are now declared in a shared header, shouldn't these be
added to their own file, newlib/libm/fenv/fe_nomask_env.c parallel
to newlib/libm/fenv/fe_dfl_env.c?

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

Please drop Cygwin-specific comments.  They just don't make sense in
common newlib code, except in rare cases to explain a difference to
other targets.

> [...]
> +#if defined(__CYGWIN__)

Great.

> +/*  Returns the currently selected precision, represented by one of the
> +   values of the defined precision macros.  */
> +int
> +fegetprec (void)
> +{
> [...]
> +int
> +fesetprec (int prec)
> +{
> [...]
> +#endif

Uh oh.  What about _feinitialise()?  Cygwin calls this function
right from the initial code, but is it really the right thing
to enforce this for all i386/x86_64 targets?

Any idea how we can generate the default environment on the fly
while maintaining backward compat on Cygwin?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: signature.asc
Description: PGP signature