diff mbox series

newlib: add dummy implementations of fe{get,set}prec for Aarch64 Cygwin

Message ID DB9PR83MB09237810256ACC7321E47D8E9243A@DB9PR83MB0923.EURPRD83.prod.outlook.com
State New
Headers
Series newlib: add dummy implementations of fe{get,set}prec for Aarch64 Cygwin |

Commit Message

Radek Barton July 3, 2025, 12:42 p.m. UTC 
  Hello.

Albeit Aarch64 does not support setting floating point operations precisions at runtime `cygwin1.dll` exports `fegetprec` and `fesetprec` functions leading to undefined references when linking. This patch adds their dummy implementations in a similar way how they are defined in `newlib/libc/machine/shared_x86/sys/fenv.h`, resp. in `/newlib/libm/machine/shared_x86/fenv.c`.

Alternative fix would be to modify `winsup/cygwin/scripts/mkimport` and `winsup/cygwin/scripts/gendef` scripts so they would exclude those from being exported to the DLL upon specific label in `winsup/cygwin/cygwin.din`, e.g.:

```
fegetprec NOSIGFE NOARM64
fesetprec NOSIGFE NOARM64
```

The same options applies to `_fe_nomask_env` which would be for the first option sent as a separate patch as it changes only `winsup` codebase.

Which one would you prefer?

Radek

---
From 0ea739b8eef37d433712c185e0562c9f3142b86c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radek=20Barto=C5=88?= <radek.barton@microsoft.com>
Date: Thu, 3 Jul 2025 12:02:03 +0200
Subject: [PATCH] newlib: add dummy implementations of fe{get,set}prec
 for Aarch64 Cygwin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Radek Bartoň <radek.barton@microsoft.com>
---
 newlib/libc/machine/aarch64/sys/fenv.h | 24 ++++++++++++++++++++-
 newlib/libm/machine/aarch64/fenv.c     | 30 ++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

Comments

Richard Earnshaw July 3, 2025, 3:10 p.m. UTC | #1 
On 03/07/2025 13:42, Radek Barton wrote:
> Hello.
> 
> Albeit Aarch64 does not support setting floating point operations precisions at runtime `cygwin1.dll` exports `fegetprec` and `fesetprec` functions leading to undefined references when linking. This patch adds their dummy implementations in a similar way how they are defined in `newlib/libc/machine/shared_x86/sys/fenv.h`, resp. in `/newlib/libm/machine/shared_x86/fenv.c`.
> 
> Alternative fix would be to modify `winsup/cygwin/scripts/mkimport` and `winsup/cygwin/scripts/gendef` scripts so they would exclude those from being exported to the DLL upon specific label in `winsup/cygwin/cygwin.din`, e.g.:
> 
> ```
> fegetprec NOSIGFE NOARM64
> fesetprec NOSIGFE NOARM64
> ```
> 
> The same options applies to `_fe_nomask_env` which would be for the first option sent as a separate patch as it changes only `winsup` codebase.
> 
> Which one would you prefer?
> 
> Radek
> 
> ---
> From 0ea739b8eef37d433712c185e0562c9f3142b86c Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Radek=20Barto=C5=88?= <radek.barton@microsoft.com>
> Date: Thu, 3 Jul 2025 12:02:03 +0200
> Subject: [PATCH] newlib: add dummy implementations of fe{get,set}prec
>  for Aarch64 Cygwin
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Signed-off-by: Radek Bartoň <radek.barton@microsoft.com>
> ---
>  newlib/libc/machine/aarch64/sys/fenv.h | 24 ++++++++++++++++++++-
>  newlib/libm/machine/aarch64/fenv.c     | 30 ++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/newlib/libc/machine/aarch64/sys/fenv.h b/newlib/libc/machine/aarch64/sys/fenv.h
> index 212612725..0e4e7f492 100644
> --- a/newlib/libc/machine/aarch64/sys/fenv.h
> +++ b/newlib/libc/machine/aarch64/sys/fenv.h
> @@ -61,7 +61,14 @@ typedef	__uint64_t	fexcept_t;
>  			 FE_UPWARD | FE_TOWARDZERO)
>  #define	_ROUND_SHIFT	22
>  
> -
> +/* 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 spec you point to says:

       Each macro

           FE_FLTPREC
           FE_DBLPREC
           FE_LDBLPREC

       is defined *if and only if* the implementation supports the
       dynamic precision by means of the functions in 7.6.z

(Emphasis mine).

Thus, since we don't support changing the precision in this way, aarch64 should not define these values.

I think it's ok to define these routines if that's really needed.  But...

>  
>  /* Default floating-point environment */
>  extern const fenv_t	*_fe_dfl_env;
> @@ -115,6 +122,21 @@ fegetexcept(void)
>  
>  #endif /* __BSD_VISIBLE */
>  
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
>  
> +#ifdef __CYGWIN__
> +
> +#if __MISC_VISIBLE
> +int fegetprec (void);
> +int fesetprec (int __prec);
> +#endif
> +
> +#endif /* __CYGWIN__ */
> +
> +#ifdef __cplusplus
> +}
> +#endif
>  
>  #endif	/* !_FENV_H_ */
> diff --git a/newlib/libm/machine/aarch64/fenv.c b/newlib/libm/machine/aarch64/fenv.c
> index 3ffe23441..3f3459e9d 100644
> --- a/newlib/libm/machine/aarch64/fenv.c
> +++ b/newlib/libm/machine/aarch64/fenv.c
> @@ -55,3 +55,33 @@ extern inline int feupdateenv(const fenv_t *__envp);
>  extern inline int feenableexcept(int __mask);
>  extern inline int fedisableexcept(int __mask);
>  extern inline int fegetexcept(void);
> +
> +#if defined(__CYGWIN__)
> +
> +/* Returns the currently selected precision, represented by one of the
> +   values of the defined precision macros. */
> +int
> +fegetprec (void)
> +{
> +  /* AArch64 doesn't have configurable precision.
> +     Return a fixed value indicating double precision (most common). */
> +  return FE_DBLPREC;
> +}
> +

I'd be more inclined to return (-1) here as that is unlikely to match any precision value that someone has hard-coded and is more indicative of an error.

> +/* 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)
> +{
> +  /* Aarch64 doesn't support changing precision at runtime. */
> +  return 0; // return failure
> +}
> +
> +#endif

R.
Corinna Vinschen July 3, 2025, 3:51 p.m. UTC | #2 
On Jul  3 12:42, Radek Barton wrote:
> Hello.
> 
> Albeit Aarch64 does not support setting floating point operations precisions at runtime `cygwin1.dll` exports `fegetprec` and `fesetprec` functions leading to undefined references when linking. This patch adds their dummy implementations in a similar way how they are defined in `newlib/libc/machine/shared_x86/sys/fenv.h`, resp. in `/newlib/libm/machine/shared_x86/fenv.c`.
> 
> Alternative fix would be to modify `winsup/cygwin/scripts/mkimport` and `winsup/cygwin/scripts/gendef` scripts so they would exclude those from being exported to the DLL upon specific label in `winsup/cygwin/cygwin.din`, e.g.:
> 
> ```
> fegetprec NOSIGFE NOARM64
> fesetprec NOSIGFE NOARM64
> ```
> 
> The same options applies to `_fe_nomask_env` which would be for the first option sent as a separate patch as it changes only `winsup` codebase.
> 
> Which one would you prefer?

What Richard said, but there's another choice and that might be
preferrable:

Add a new file to the winsup/cygwin/aarch64 subdir, like, say, feprec.c,
and implement those dummy functions there.  This keeps the newlib
file clean and should work nicely.


Thanks,
Corinna
diff mbox series

Patch

diff --git a/newlib/libc/machine/aarch64/sys/fenv.h b/newlib/libc/machine/aarch64/sys/fenv.h
index 212612725..0e4e7f492 100644
--- a/newlib/libc/machine/aarch64/sys/fenv.h
+++ b/newlib/libc/machine/aarch64/sys/fenv.h
@@ -61,7 +61,14 @@  typedef	__uint64_t	fexcept_t;
 			 FE_UPWARD | FE_TOWARDZERO)
 #define	_ROUND_SHIFT	22
 
-
+/* 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
 
 /* Default floating-point environment */
 extern const fenv_t	*_fe_dfl_env;
@@ -115,6 +122,21 @@  fegetexcept(void)
 
 #endif /* __BSD_VISIBLE */
 
+#ifdef __cplusplus
+extern "C" {
+#endif
 
+#ifdef __CYGWIN__
+
+#if __MISC_VISIBLE
+int fegetprec (void);
+int fesetprec (int __prec);
+#endif
+
+#endif /* __CYGWIN__ */
+
+#ifdef __cplusplus
+}
+#endif
 
 #endif	/* !_FENV_H_ */
diff --git a/newlib/libm/machine/aarch64/fenv.c b/newlib/libm/machine/aarch64/fenv.c
index 3ffe23441..3f3459e9d 100644
--- a/newlib/libm/machine/aarch64/fenv.c
+++ b/newlib/libm/machine/aarch64/fenv.c
@@ -55,3 +55,33 @@  extern inline int feupdateenv(const fenv_t *__envp);
 extern inline int feenableexcept(int __mask);
 extern inline int fedisableexcept(int __mask);
 extern inline int fegetexcept(void);
+
+#if defined(__CYGWIN__)
+
+/* Returns the currently selected precision, represented by one of the
+   values of the defined precision macros. */
+int
+fegetprec (void)
+{
+  /* AArch64 doesn't have configurable precision.
+     Return a fixed value indicating double precision (most common). */
+  return FE_DBLPREC;
+}
+
+/* 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)
+{
+  /* Aarch64 doesn't support changing precision at runtime. */
+  return 0; // return failure
+}
+
+#endif