public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] sparc: Do not test preservation of NaN payloads for LEON
@ 2024-01-12  9:26 Daniel Cederman
  2024-01-12  9:26 ` [PATCH] sparc: Prevent stfsr from directly following floating-point instruction Daniel Cederman
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Daniel Cederman @ 2024-01-12  9:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: daniel, andreas

The FPU used by LEON does not preserve NaN payload. This change allows
the math/test-*-canonicalize tests to pass on LEON.

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 .../sparc32/fpu/math-tests-snan-payload.h     | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h

diff --git a/sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h b/sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h
new file mode 100644
index 0000000000..d84e4d997b
--- /dev/null
+++ b/sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h
@@ -0,0 +1,30 @@
+/* Configuration for math tests: sNaN payloads.  SPARC version.
+   Copyright (C) 2016-2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef SPARC_MATH_TESTS_SNAN_PAYLOAD_H
+#define SPARC_MATH_TESTS_SNAN_PAYLOAD_H 1
+
+/* LEON floating-point instructions do not preserve sNaN
+   payloads.  */
+#if defined (__leon__)
+# define SNAN_TESTS_PRESERVE_PAYLOAD	0
+#else
+# define SNAN_TESTS_PRESERVE_PAYLOAD	1
+#endif
+
+#endif /* math-tests-snan-payload.h.  */
-- 
2.40.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] sparc: Prevent stfsr from directly following floating-point instruction
  2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
@ 2024-01-12  9:26 ` Daniel Cederman
  2024-01-12 14:38   ` Adhemerval Zanella Netto
  2024-01-12  9:26 ` [PATCH] sparc: Force calculation that raises exception Daniel Cederman
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-12  9:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: daniel, andreas

On LEON, if the stfsr instruction is immediately following a floating-point
operation instruction in a running program, with no other instruction in
between the two, the stfsr might behave as if the order was reversed
between the two instructions and the stfsr occurred before the
floating-point operation.

Add a nop instruction before the stfsr to prevent this from happening.

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 sysdeps/sparc/fpu/fenv_private.h | 6 +++++-
 sysdeps/sparc/fpu/fpu_control.h  | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/sysdeps/sparc/fpu/fenv_private.h b/sysdeps/sparc/fpu/fenv_private.h
index da7c7fe332..a02af80d04 100644
--- a/sysdeps/sparc/fpu/fenv_private.h
+++ b/sysdeps/sparc/fpu/fenv_private.h
@@ -8,7 +8,11 @@
 # define __fenv_stfsr(X)   __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (X))
 # define __fenv_ldfsr(X)   __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (X))
 #else
-# define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
+# ifdef __leon__
+#  define __fenv_stfsr(X)   __asm__ __volatile__ ("nop; st %%fsr,%0" : "=m" (X))
+# else
+#  define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
+# endif
 # define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
 #endif
 
diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
index dd18789573..9313743f86 100644
--- a/sysdeps/sparc/fpu/fpu_control.h
+++ b/sysdeps/sparc/fpu/fpu_control.h
@@ -61,7 +61,11 @@ typedef unsigned long int fpu_control_t;
 # define _FPU_GETCW(cw) __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (*&cw))
 # define _FPU_SETCW(cw) __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (*&cw))
 #else
-# define _FPU_GETCW(cw) __asm__ __volatile__ ("st %%fsr,%0" : "=m" (*&cw))
+# ifdef __leon__
+#  define _FPU_GETCW(cw) __asm__ __volatile__ ("nop; st %%fsr,%0" : "=m" (*&cw))
+# else
+#  define _FPU_GETCW(cw) __asm__ __volatile__ ("st %%fsr,%0" : "=m" (*&cw))
+# endif
 # define _FPU_SETCW(cw) __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (*&cw))
 #endif
 
-- 
2.40.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] sparc: Force calculation that raises exception
  2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
  2024-01-12  9:26 ` [PATCH] sparc: Prevent stfsr from directly following floating-point instruction Daniel Cederman
@ 2024-01-12  9:26 ` Daniel Cederman
  2024-01-12 16:45   ` Adhemerval Zanella Netto
  2024-01-12  9:26 ` [PATCH] sparc: Treat the version field in the FPU control word as reserved Daniel Cederman
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-12  9:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: daniel, andreas

Read out the FPU control word to force the calculation to complete and
raise the exception.

With this change the math/test-fenv test pass for LEON.

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 sysdeps/sparc/fpu/fraiseexcpt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sysdeps/sparc/fpu/fraiseexcpt.c b/sysdeps/sparc/fpu/fraiseexcpt.c
index 26a7720ec9..43176cf6a3 100644
--- a/sysdeps/sparc/fpu/fraiseexcpt.c
+++ b/sysdeps/sparc/fpu/fraiseexcpt.c
@@ -20,6 +20,7 @@
 #include <float.h>
 #include <math.h>
 #include <shlib-compat.h>
+#include <fpu_control.h>
 
 int
 __feraiseexcept (int excepts)
@@ -30,6 +31,7 @@ __feraiseexcept (int excepts)
     0.0, 1.0, DBL_MAX, DBL_MIN, M_PI
   };
   double d;
+  fpu_control_t cw;
 
   /* Raise exceptions represented by EXPECTS.  But we must raise only
      one signal at a time.  It is important the if the overflow/underflow
@@ -43,6 +45,7 @@ __feraiseexcept (int excepts)
       __asm ("" : "=e" (d) : "0" (c.zero));
       d /= c.zero;
       __asm __volatile ("" : : "e" (d));
+      _FPU_GETCW(cw);
     }
 
   /* Next: division by zero.  */
@@ -51,6 +54,7 @@ __feraiseexcept (int excepts)
       __asm ("" : "=e" (d) : "0" (c.one));
       d /= c.zero;
       __asm __volatile ("" : : "e" (d));
+      _FPU_GETCW(cw);
     }
 
   /* Next: overflow.  */
@@ -59,6 +63,7 @@ __feraiseexcept (int excepts)
       __asm ("" : "=e" (d) : "0" (c.max));
       d *= d;
       __asm __volatile ("" : : "e" (d));
+      _FPU_GETCW(cw);
     }
 
   /* Next: underflow.  */
@@ -67,6 +72,7 @@ __feraiseexcept (int excepts)
       __asm ("" : "=e" (d) : "0" (c.min));
       d *= d;
       __asm __volatile ("" : : "e" (d));
+      _FPU_GETCW(cw);
     }
 
   /* Last: inexact.  */
@@ -75,6 +81,7 @@ __feraiseexcept (int excepts)
       __asm ("" : "=e" (d) : "0" (c.one));
       d /= c.pi;
       __asm __volatile ("" : : "e" (d));
+      _FPU_GETCW(cw);
     }
 
   /* Success.  */
-- 
2.40.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] sparc: Treat the version field in the FPU control word as reserved
  2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
  2024-01-12  9:26 ` [PATCH] sparc: Prevent stfsr from directly following floating-point instruction Daniel Cederman
  2024-01-12  9:26 ` [PATCH] sparc: Force calculation that raises exception Daniel Cederman
@ 2024-01-12  9:26 ` Daniel Cederman
  2024-01-12 17:42   ` Adhemerval Zanella Netto
  2024-01-12  9:26 ` [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32 Daniel Cederman
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-12  9:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: daniel, andreas

The FSR version field is read-only and might be non-zero.

This allows math/test-fpucw* to correctly pass when the version is
non-zero.

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 sysdeps/sparc/fpu/fpu_control.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
index 9313743f86..36a2bf5d07 100644
--- a/sysdeps/sparc/fpu/fpu_control.h
+++ b/sysdeps/sparc/fpu/fpu_control.h
@@ -42,7 +42,7 @@
 #define _FPU_RC_ZERO    0x40000000
 #define _FPU_RC_NEAREST 0x0        /* RECOMMENDED */
 
-#define _FPU_RESERVED   0x30300000  /* Reserved bits in cw */
+#define _FPU_RESERVED   0x303e0000  /* Reserved bits in cw */
 
 
 /* Now two recommended cw */
-- 
2.40.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32
  2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
                   ` (2 preceding siblings ...)
  2024-01-12  9:26 ` [PATCH] sparc: Treat the version field in the FPU control word as reserved Daniel Cederman
@ 2024-01-12  9:26 ` Daniel Cederman
  2024-01-12 18:05   ` Adhemerval Zanella Netto
  2024-01-12  9:26 ` [PATCH] sparc: Remove unwind information from signal return stub functions Daniel Cederman
  2024-01-16 15:37 ` [PATCH] sparc: Do not test preservation of NaN payloads for LEON Adhemerval Zanella Netto
  5 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-12  9:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: daniel, andreas

Conversions from a float to a long long on 32-bit SPARC may not
raise the correct exceptions on overflow. It also may raise spurious
"inexact" exceptions on non overflow cases. This patch fixes the
problem in the same way as for RV32.

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 .../sparc32/fpu/fix-fp-int-convert-overflow.h | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h

diff --git a/sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h b/sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h
new file mode 100644
index 0000000000..ae79a106e7
--- /dev/null
+++ b/sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h
@@ -0,0 +1,35 @@
+/* Fix for conversion of floating point to integer overflow.  SPARC32 version.
+   Copyright (C) 2015-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef FIX_FP_INT_CONVERT_OVERFLOW_H
+#define FIX_FP_INT_CONVERT_OVERFLOW_H	1
+
+/* Define these macros to 1 to workaround conversions of out-of-range
+   floating-point numbers to integer types failing to raise the
+   "invalid" exception, or raising spurious "inexact" or other
+   exceptions.  */
+#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
+#define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
+#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
+#define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
+#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
+#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
+#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
+#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
+
+#endif /* fix-fp-int-convert-overflow.h */
-- 
2.40.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] sparc: Remove unwind information from signal return stub functions
  2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
                   ` (3 preceding siblings ...)
  2024-01-12  9:26 ` [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32 Daniel Cederman
@ 2024-01-12  9:26 ` Daniel Cederman
  2024-01-15 13:41   ` Adhemerval Zanella Netto
  2024-01-16 15:37 ` [PATCH] sparc: Do not test preservation of NaN payloads for LEON Adhemerval Zanella Netto
  5 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-12  9:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: daniel, andreas

The functions were previously written in C, but were not compiled
with unwind information. The ENTRY/END macros includes .cfi_startproc
and .cfi_endproc which adds unwind information. This caused the
tests cleanup-8 and cleanup-10 in the GCC testsuite to fail.
This patch adds a version of the ENTRY/END macros without the
CFI instructions that can be used instead.

sigaction registers a restorer address that is located two instructions
before the stub function. This patch adds a two instruction padding to
avoid that the unwinder accesses the unwind information from the function
that the linker has placed right before it in memory. This fixes an issue
with pthread_cancel that caused tst-mutex8-static (and other tests) to fail.

This patch fixes the issues I have seen, but I am not sure if this is
the right solution, so any feedback is welcome!

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 sysdeps/sparc/sysdep.h                                |  9 +++++++++
 .../unix/sysv/linux/sparc/sparc32/sigreturn_stub.S    | 11 +++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/sysdeps/sparc/sysdep.h b/sysdeps/sparc/sysdep.h
index c00fe79fec..44a6952bff 100644
--- a/sysdeps/sparc/sysdep.h
+++ b/sysdeps/sparc/sysdep.h
@@ -76,6 +76,15 @@ C_LABEL(name)				\
 	cfi_endproc;			\
 	.size name, . - name
 
+#define ENTRY_NOCFI(name)			\
+	.align	4;			\
+	.global	C_SYMBOL_NAME(name);	\
+	.type	name, @function;	\
+C_LABEL(name)
+
+#define END_NOCFI(name)			\
+	.size name, . - name
+
 #undef LOC
 #define LOC(name)  .L##name
 
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S b/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S
index 832223f8ce..21d36c50df 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S
@@ -23,12 +23,15 @@
 
    [1] https://lkml.org/lkml/2016/5/27/465  */
 
-ENTRY (__rt_sigreturn_stub)
+	nop
+	nop
+
+ENTRY_NOCFI (__rt_sigreturn_stub)
 	mov	__NR_rt_sigreturn, %g1
 	ta	0x10
-END (__rt_sigreturn_stub)
+END_NOCFI (__rt_sigreturn_stub)
 
-ENTRY (__sigreturn_stub)
+ENTRY_NOCFI (__sigreturn_stub)
 	mov	__NR_sigreturn, %g1
 	ta	0x10
-END (__sigreturn_stub)
+END_NOCFI (__sigreturn_stub)
-- 
2.40.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Prevent stfsr from directly following floating-point instruction
  2024-01-12  9:26 ` [PATCH] sparc: Prevent stfsr from directly following floating-point instruction Daniel Cederman
@ 2024-01-12 14:38   ` Adhemerval Zanella Netto
  2024-01-15  9:37     ` Daniel Cederman
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-12 14:38 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 12/01/24 06:26, Daniel Cederman wrote:
> On LEON, if the stfsr instruction is immediately following a floating-point
> operation instruction in a running program, with no other instruction in
> between the two, the stfsr might behave as if the order was reversed
> between the two instructions and the stfsr occurred before the
> floating-point operation.
> 
> Add a nop instruction before the stfsr to prevent this from happening.
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>

You might want to check if __builtin_store_fsr gcc builtin is also subject
to this issue (it used on atomic floating point support as well).

> ---
>  sysdeps/sparc/fpu/fenv_private.h | 6 +++++-
>  sysdeps/sparc/fpu/fpu_control.h  | 6 +++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/sparc/fpu/fenv_private.h b/sysdeps/sparc/fpu/fenv_private.h
> index da7c7fe332..a02af80d04 100644
> --- a/sysdeps/sparc/fpu/fenv_private.h
> +++ b/sysdeps/sparc/fpu/fenv_private.h
> @@ -8,7 +8,11 @@
>  # define __fenv_stfsr(X)   __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (X))
>  # define __fenv_ldfsr(X)   __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (X))
>  #else
> -# define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
> +# ifdef __leon__
> +#  define __fenv_stfsr(X)   __asm__ __volatile__ ("nop; st %%fsr,%0" : "=m" (X))
> +# else
> +#  define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
> +# endif
>  # define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
>  #endif
>  

I think it would be better to use _FPU_GETCW/_FPU_SETCW here, this will me
the fix only required at fpu_control.h:

#include <fpu_control.h>
[...]
#define __fenv_stfsr(X)   _FPU_GETCW (X)
#define __fenv_ldfsr(X)   _FPU_SETCW (X)
[...]

> diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
> index dd18789573..9313743f86 100644
> --- a/sysdeps/sparc/fpu/fpu_control.h
> +++ b/sysdeps/sparc/fpu/fpu_control.h
> @@ -61,7 +61,11 @@ typedef unsigned long int fpu_control_t;
>  # define _FPU_GETCW(cw) __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (*&cw))
>  # define _FPU_SETCW(cw) __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (*&cw))
>  #else
> -# define _FPU_GETCW(cw) __asm__ __volatile__ ("st %%fsr,%0" : "=m" (*&cw))
> +# ifdef __leon__

Please also add a brief comment on why the nop is required.

> +#  define _FPU_GETCW(cw) __asm__ __volatile__ ("nop; st %%fsr,%0" : "=m" (*&cw))
> +# else
> +#  define _FPU_GETCW(cw) __asm__ __volatile__ ("st %%fsr,%0" : "=m" (*&cw))
> +# endif
>  # define _FPU_SETCW(cw) __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (*&cw))
>  #endif
>  

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Force calculation that raises exception
  2024-01-12  9:26 ` [PATCH] sparc: Force calculation that raises exception Daniel Cederman
@ 2024-01-12 16:45   ` Adhemerval Zanella Netto
  2024-01-15  9:41     ` Daniel Cederman
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-12 16:45 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 12/01/24 06:26, Daniel Cederman wrote:
> Read out the FPU control word to force the calculation to complete and
> raise the exception.
> 
> With this change the math/test-fenv test pass for LEON.

Is this to try mitigate 'GRFPU Floating-point controller: Missing 
FDIV/FSQRT Result' [1]?

If so, wouldn't be better to use '-mfix-ut699' when building against
leon3? There are potentially multiple usages of FDIV/FSQRT within 
libc/libm that might be subject to this issue.

At least recent gcc versions seems to add the required nops after fsqrt:

$ cat t.c
#include <math.h>
#include <float.h>

#define math_force_eval(x)                                              \
  ({ __typeof (x) __x = (x); __asm __volatile__ ("" : : "m" (__x)); })

int foo (void)
{
  static const struct {
    double zero, one, max, min, pi;
  } c = {
    0.0, 1.0, DBL_MAX, DBL_MIN, M_PI
  };
  double d;
  asm ("" : "=e" (d) : "0" (c.zero));
  d /= c.zero;
  __asm __volatile ("" : : "e" (d));
}
$ sparc64-linux-gnu -m32 -mcpu=leon3 -O2 -mhard-float -mfix-ut699 t.c -S -o -
[...]
foo:
        sethi   %hi(.LC0), %g1
        ldd     [%g1+%lo(.LC0)], %f10
        fmovs   %f10, %f8
        fmovs   %f11, %f9
        fdivd   %f8, %f10, %f8
        std     %f8, [%sp-8]
        nop
        nop
        jmp     %o7+8
[...]

In any case this penalize non-leon3 chips with the extra 'stx fsr,...', so
it should be used only for leon (and with a proper explanation of why it is
required).

[1] https://www.gaisler.com/doc/antn/GRLIB-TN-0013.pdf

> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> ---
>  sysdeps/sparc/fpu/fraiseexcpt.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sysdeps/sparc/fpu/fraiseexcpt.c b/sysdeps/sparc/fpu/fraiseexcpt.c
> index 26a7720ec9..43176cf6a3 100644
> --- a/sysdeps/sparc/fpu/fraiseexcpt.c
> +++ b/sysdeps/sparc/fpu/fraiseexcpt.c
> @@ -20,6 +20,7 @@
>  #include <float.h>
>  #include <math.h>
>  #include <shlib-compat.h>
> +#include <fpu_control.h>
>  
>  int
>  __feraiseexcept (int excepts)
> @@ -30,6 +31,7 @@ __feraiseexcept (int excepts)
>      0.0, 1.0, DBL_MAX, DBL_MIN, M_PI
>    };
>    double d;
> +  fpu_control_t cw;
>  
>    /* Raise exceptions represented by EXPECTS.  But we must raise only
>       one signal at a time.  It is important the if the overflow/underflow
> @@ -43,6 +45,7 @@ __feraiseexcept (int excepts)
>        __asm ("" : "=e" (d) : "0" (c.zero));
>        d /= c.zero;
>        __asm __volatile ("" : : "e" (d));
> +      _FPU_GETCW(cw);
>      }
>  
>    /* Next: division by zero.  */
> @@ -51,6 +54,7 @@ __feraiseexcept (int excepts)
>        __asm ("" : "=e" (d) : "0" (c.one));
>        d /= c.zero;
>        __asm __volatile ("" : : "e" (d));
> +      _FPU_GETCW(cw);
>      }
>  
>    /* Next: overflow.  */
> @@ -59,6 +63,7 @@ __feraiseexcept (int excepts)
>        __asm ("" : "=e" (d) : "0" (c.max));
>        d *= d;
>        __asm __volatile ("" : : "e" (d));
> +      _FPU_GETCW(cw);
>      }
>  
>    /* Next: underflow.  */
> @@ -67,6 +72,7 @@ __feraiseexcept (int excepts)
>        __asm ("" : "=e" (d) : "0" (c.min));
>        d *= d;
>        __asm __volatile ("" : : "e" (d));
> +      _FPU_GETCW(cw);
>      }
>  
>    /* Last: inexact.  */
> @@ -75,6 +81,7 @@ __feraiseexcept (int excepts)
>        __asm ("" : "=e" (d) : "0" (c.one));
>        d /= c.pi;
>        __asm __volatile ("" : : "e" (d));
> +      _FPU_GETCW(cw);
>      }
>  
>    /* Success.  */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Treat the version field in the FPU control word as reserved
  2024-01-12  9:26 ` [PATCH] sparc: Treat the version field in the FPU control word as reserved Daniel Cederman
@ 2024-01-12 17:42   ` Adhemerval Zanella Netto
  2024-02-15  9:31     ` Daniel Cederman
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-12 17:42 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 12/01/24 06:26, Daniel Cederman wrote:
> The FSR version field is read-only and might be non-zero.
> 
> This allows math/test-fpucw* to correctly pass when the version is
> non-zero.
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>

It looks reasonable to mask off the version _FPU_RESERVED.  It also
means that it would be change by __setfpucw, although afaik the ISA
guarantee that it can not be changed anyway.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/sparc/fpu/fpu_control.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
> index 9313743f86..36a2bf5d07 100644
> --- a/sysdeps/sparc/fpu/fpu_control.h
> +++ b/sysdeps/sparc/fpu/fpu_control.h
> @@ -42,7 +42,7 @@
>  #define _FPU_RC_ZERO    0x40000000
>  #define _FPU_RC_NEAREST 0x0        /* RECOMMENDED */
>  
> -#define _FPU_RESERVED   0x30300000  /* Reserved bits in cw */
> +#define _FPU_RESERVED   0x303e0000  /* Reserved bits in cw */
>  
>  
>  /* Now two recommended cw */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32
  2024-01-12  9:26 ` [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32 Daniel Cederman
@ 2024-01-12 18:05   ` Adhemerval Zanella Netto
  2024-01-15 14:38     ` Daniel Cederman
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-12 18:05 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 12/01/24 06:26, Daniel Cederman wrote:
> Conversions from a float to a long long on 32-bit SPARC may not
> raise the correct exceptions on overflow. It also may raise spurious
> "inexact" exceptions on non overflow cases. This patch fixes the
> problem in the same way as for RV32.
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> ---
>  .../sparc32/fpu/fix-fp-int-convert-overflow.h | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h
> 
> diff --git a/sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h b/sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h
> new file mode 100644
> index 0000000000..ae79a106e7
> --- /dev/null
> +++ b/sysdeps/sparc/sparc32/fpu/fix-fp-int-convert-overflow.h
> @@ -0,0 +1,35 @@
> +/* Fix for conversion of floating point to integer overflow.  SPARC32 version.
> +   Copyright (C) 2015-2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef FIX_FP_INT_CONVERT_OVERFLOW_H
> +#define FIX_FP_INT_CONVERT_OVERFLOW_H	1
> +
> +/* Define these macros to 1 to workaround conversions of out-of-range
> +   floating-point numbers to integer types failing to raise the
> +   "invalid" exception, or raising spurious "inexact" or other
> +   exceptions.  */

This is not really true for all sparc32, at least with sparcv9 I have
access I have not seen any issues with missing exception overflows or
spurious inexact.  Are these some leon3 limitation or issue?

I think we need to know exactly how/when this happens and only enable
such workaound when required.

> +#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
> +#define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
> +#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
> +#define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
> +#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
> +#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
> +#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
> +#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
> +
> +#endif /* fix-fp-int-convert-overflow.h */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Prevent stfsr from directly following floating-point instruction
  2024-01-12 14:38   ` Adhemerval Zanella Netto
@ 2024-01-15  9:37     ` Daniel Cederman
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Cederman @ 2024-01-15  9:37 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha; +Cc: daniel, andreas


On 2024-01-12 15:38, Adhemerval Zanella Netto wrote:
> 
> 
> On 12/01/24 06:26, Daniel Cederman wrote:
>> On LEON, if the stfsr instruction is immediately following a floating-point
>> operation instruction in a running program, with no other instruction in
>> between the two, the stfsr might behave as if the order was reversed
>> between the two instructions and the stfsr occurred before the
>> floating-point operation.
>>
>> Add a nop instruction before the stfsr to prevent this from happening.
>>
>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> 
> You might want to check if __builtin_store_fsr gcc builtin is also subject
> to this issue (it used on atomic floating point support as well).
> 

Thank you for reviewing the patches! I will address your comments and 
send an updated version. I was not aware of the gcc builtin for fsr so 
that is likely something that needs to be fixed. Thanks!

/Daniel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Force calculation that raises exception
  2024-01-12 16:45   ` Adhemerval Zanella Netto
@ 2024-01-15  9:41     ` Daniel Cederman
  2024-01-15 11:47       ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-15  9:41 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha; +Cc: daniel, andreas

On 2024-01-12 17:45, Adhemerval Zanella Netto wrote:
> 
> 
> On 12/01/24 06:26, Daniel Cederman wrote:
>> Read out the FPU control word to force the calculation to complete and
>> raise the exception.
>>
>> With this change the math/test-fenv test pass for LEON.
> 
> Is this to try mitigate 'GRFPU Floating-point controller: Missing
> FDIV/FSQRT Result' [1]?
> 

No, this change is not for any errata. The current implementation 
generates code that looks like this:

   ac:	91 a2 09 ca 	fdivd  %f8, %f10, %f8
   b0:	90 10 20 00 	clr  %o0
   b4:	81 c3 e0 08 	retl
   bc:	9c 03 a0 50 	add  %sp, 0x50, %sp

The division performed will raise an exception, but if the FPU is 
running asynchronous with the CPU the exception will not happen until 
the division is finished and another fp operation accesses the FPU. 
Reading out the control word forces the division to finish and the 
exception to trigger.

But looking at it now I think this is the more correct change:

- __asm __volatile ("" : : "e" (d));
+ __asm __volatile ("" : : "m" (d));

That would generate a store instruction with the same effect as the stfsr.

/Daniel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Force calculation that raises exception
  2024-01-15  9:41     ` Daniel Cederman
@ 2024-01-15 11:47       ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-15 11:47 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 15/01/24 06:41, Daniel Cederman wrote:
> On 2024-01-12 17:45, Adhemerval Zanella Netto wrote:
>>
>>
>> On 12/01/24 06:26, Daniel Cederman wrote:
>>> Read out the FPU control word to force the calculation to complete and
>>> raise the exception.
>>>
>>> With this change the math/test-fenv test pass for LEON.
>>
>> Is this to try mitigate 'GRFPU Floating-point controller: Missing
>> FDIV/FSQRT Result' [1]?
>>
> 
> No, this change is not for any errata. The current implementation generates code that looks like this:
> 
>   ac:    91 a2 09 ca     fdivd  %f8, %f10, %f8
>   b0:    90 10 20 00     clr  %o0
>   b4:    81 c3 e0 08     retl
>   bc:    9c 03 a0 50     add  %sp, 0x50, %sp
> 
> The division performed will raise an exception, but if the FPU is running asynchronous with the CPU the exception will not happen until the division is finished and another fp operation accesses the FPU. Reading out the control word forces the division to finish and the exception to trigger.
> 
> But looking at it now I think this is the more correct change:
> 
> - __asm __volatile ("" : : "e" (d));
> + __asm __volatile ("" : : "m" (d));
> 
> That would generate a store instruction with the same effect as the stfsr.

We have the math_force_eval macro exactly for this [1].  It should be ok
to be used on the generic implementation (and not making it leon3 specific).

[1] https://godbolt.org/z/sPWKhhnsz

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Remove unwind information from signal return stub functions
  2024-01-12  9:26 ` [PATCH] sparc: Remove unwind information from signal return stub functions Daniel Cederman
@ 2024-01-15 13:41   ` Adhemerval Zanella Netto
  2024-01-15 14:22     ` Daniel Cederman
  0 siblings, 1 reply; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-15 13:41 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 12/01/24 06:26, Daniel Cederman wrote:
> The functions were previously written in C, but were not compiled
> with unwind information. The ENTRY/END macros includes .cfi_startproc
> and .cfi_endproc which adds unwind information. This caused the
> tests cleanup-8 and cleanup-10 in the GCC testsuite to fail.
> This patch adds a version of the ENTRY/END macros without the
> CFI instructions that can be used instead.
> 
> sigaction registers a restorer address that is located two instructions
> before the stub function. This patch adds a two instruction padding to
> avoid that the unwinder accesses the unwind information from the function
> that the linker has placed right before it in memory. This fixes an issue
> with pthread_cancel that caused tst-mutex8-static (and other tests) to fail.
> 
> This patch fixes the issues I have seen, but I am not sure if this is
> the right solution, so any feedback is welcome!
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>

I am not sure this is the correct fix, at least on a sparc machine I 
do not any improvements on a sparcv9 build. But do I agree that this
seems to be a wrong/missing unwind information for the signal return
stub: the tst-mutex8-static does seems be on an infinite loop in the
libgcc unwinder code.

Does it help to restore the stub to be coded in C? It seems that the
problem that lead to rewrite the stubs in assembly (b33e946fbb1659d)
is mainly caused by '-fPIE/-fpic/-fPIC' which seems to generate a
stack frame.  I see it has started with gcc 8, and I am not sure if
this is really required by sparc ABI or it is something related to
to other compiler flags we are using (I tried to use
-fno-asynchronous-unwind-tables -fno-exceptions to prevent it, but
without much success).


> ---
>  sysdeps/sparc/sysdep.h                                |  9 +++++++++
>  .../unix/sysv/linux/sparc/sparc32/sigreturn_stub.S    | 11 +++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/sparc/sysdep.h b/sysdeps/sparc/sysdep.h
> index c00fe79fec..44a6952bff 100644
> --- a/sysdeps/sparc/sysdep.h
> +++ b/sysdeps/sparc/sysdep.h
> @@ -76,6 +76,15 @@ C_LABEL(name)				\
>  	cfi_endproc;			\
>  	.size name, . - name
>  
> +#define ENTRY_NOCFI(name)			\
> +	.align	4;			\
> +	.global	C_SYMBOL_NAME(name);	\
> +	.type	name, @function;	\
> +C_LABEL(name)
> +
> +#define END_NOCFI(name)			\
> +	.size name, . - name
> +
>  #undef LOC
>  #define LOC(name)  .L##name
>  
> diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S b/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S
> index 832223f8ce..21d36c50df 100644
> --- a/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S
> +++ b/sysdeps/unix/sysv/linux/sparc/sparc32/sigreturn_stub.S
> @@ -23,12 +23,15 @@
>  
>     [1] https://lkml.org/lkml/2016/5/27/465  */
>  
> -ENTRY (__rt_sigreturn_stub)
> +	nop
> +	nop
> +
> +ENTRY_NOCFI (__rt_sigreturn_stub)
>  	mov	__NR_rt_sigreturn, %g1
>  	ta	0x10
> -END (__rt_sigreturn_stub)
> +END_NOCFI (__rt_sigreturn_stub)
>  
> -ENTRY (__sigreturn_stub)
> +ENTRY_NOCFI (__sigreturn_stub)
>  	mov	__NR_sigreturn, %g1
>  	ta	0x10
> -END (__sigreturn_stub)
> +END_NOCFI (__sigreturn_stub)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Remove unwind information from signal return stub functions
  2024-01-15 13:41   ` Adhemerval Zanella Netto
@ 2024-01-15 14:22     ` Daniel Cederman
  2024-01-15 16:57       ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-15 14:22 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha; +Cc: daniel, andreas

On 2024-01-15 14:41, Adhemerval Zanella Netto wrote:
> 
> 
> On 12/01/24 06:26, Daniel Cederman wrote:
>> The functions were previously written in C, but were not compiled
>> with unwind information. The ENTRY/END macros includes .cfi_startproc
>> and .cfi_endproc which adds unwind information. This caused the
>> tests cleanup-8 and cleanup-10 in the GCC testsuite to fail.
>> This patch adds a version of the ENTRY/END macros without the
>> CFI instructions that can be used instead.
>>
>> sigaction registers a restorer address that is located two instructions
>> before the stub function. This patch adds a two instruction padding to
>> avoid that the unwinder accesses the unwind information from the function
>> that the linker has placed right before it in memory. This fixes an issue
>> with pthread_cancel that caused tst-mutex8-static (and other tests) to fail.
>>
>> This patch fixes the issues I have seen, but I am not sure if this is
>> the right solution, so any feedback is welcome!
>>
>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> 
> I am not sure this is the correct fix, at least on a sparc machine I
> do not any improvements on a sparcv9 build. But do I agree that this

With sparcv9 build, do you mean a 32-bit or 64-bit build? The patch only 
targeted the 32-bit version of the stubs and it has been working fine 
for me on both hardware and in QEMU.

> seems to be a wrong/missing unwind information for the signal return
> stub: the tst-mutex8-static does seems be on an infinite loop in the
> libgcc unwinder code.
> 
> Does it help to restore the stub to be coded in C? It seems that the

Yes, that helps. And the only difference I noticed was that the assembly 
version had unwind information while the C version did not.

> problem that lead to rewrite the stubs in assembly (b33e946fbb1659d)
> is mainly caused by '-fPIE/-fpic/-fPIC' which seems to generate a
> stack frame.  I see it has started with gcc 8, and I am not sure if
> this is really required by sparc ABI or it is something related to
> to other compiler flags we are using (I tried to use
> -fno-asynchronous-unwind-tables -fno-exceptions to prevent it, but
> without much success).
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32
  2024-01-12 18:05   ` Adhemerval Zanella Netto
@ 2024-01-15 14:38     ` Daniel Cederman
  2024-01-15 17:52       ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-01-15 14:38 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha; +Cc: daniel, andreas

On 2024-01-12 19:05, Adhemerval Zanella Netto wrote:
> 
> 
> On 12/01/24 06:26, Daniel Cederman wrote:
>> +/* Define these macros to 1 to workaround conversions of out-of-range
>> +   floating-point numbers to integer types failing to raise the
>> +   "invalid" exception, or raising spurious "inexact" or other
>> +   exceptions.  */
> 
> This is not really true for all sparc32, at least with sparcv9 I have
> access I have not seen any issues with missing exception overflows or
> spurious inexact.  Are these some leon3 limitation or issue?
> 

I don't think it is a leon specific issue. I can trigger the same issue 
with QEMU for other sparcs. Sparcv9 has custom versions of the functions 
involved so that is probably why it is not affected:

./sysdeps/sparc/sparc32/sparcv9/fpu/multiarch/s_llrint.c
./sysdeps/sparc/sparc32/sparcv9/fpu/multiarch/s_llrintf.c

Corresponding patches for other 32-bit architectures references this 
bugzilla entry:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59412

I will have to look into this again, but I remember that it was a 
similar issue for sparc32.

> I think we need to know exactly how/when this happens and only enable
> such workaound when required.
> 
>> +#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
>> +#define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
>> +#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
>> +#define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
>> +#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
>> +#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
>> +#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
>> +#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
>> +
>> +#endif /* fix-fp-int-convert-overflow.h */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Remove unwind information from signal return stub functions
  2024-01-15 14:22     ` Daniel Cederman
@ 2024-01-15 16:57       ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-15 16:57 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 15/01/24 11:22, Daniel Cederman wrote:
> On 2024-01-15 14:41, Adhemerval Zanella Netto wrote:
>>
>>
>> On 12/01/24 06:26, Daniel Cederman wrote:
>>> The functions were previously written in C, but were not compiled
>>> with unwind information. The ENTRY/END macros includes .cfi_startproc
>>> and .cfi_endproc which adds unwind information. This caused the
>>> tests cleanup-8 and cleanup-10 in the GCC testsuite to fail.
>>> This patch adds a version of the ENTRY/END macros without the
>>> CFI instructions that can be used instead.
>>>
>>> sigaction registers a restorer address that is located two instructions
>>> before the stub function. This patch adds a two instruction padding to
>>> avoid that the unwinder accesses the unwind information from the function
>>> that the linker has placed right before it in memory. This fixes an issue
>>> with pthread_cancel that caused tst-mutex8-static (and other tests) to fail.
>>>
>>> This patch fixes the issues I have seen, but I am not sure if this is
>>> the right solution, so any feedback is welcome!
>>>
>>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
>>
>> I am not sure this is the correct fix, at least on a sparc machine I
>> do not any improvements on a sparcv9 build. But do I agree that this
> 
> With sparcv9 build, do you mean a 32-bit or 64-bit build? The patch only targeted the 32-bit version of the stubs and it has been working fine for me on both hardware and in QEMU.
> 
>> seems to be a wrong/missing unwind information for the signal return
>> stub: the tst-mutex8-static does seems be on an infinite loop in the
>> libgcc unwinder code.
>>
>> Does it help to restore the stub to be coded in C? It seems that the
> 
> Yes, that helps. And the only difference I noticed was that the assembly version had unwind information while the C version did not.
> 

In fact it was a mistake from my part while testing it on sparcv9 32 bits,
with this patch apply it does seems to fix the failures:

FAIL: debug/tst-backtrace4
FAIL: debug/tst-backtrace5
FAIL: nptl/tst-cancel24-static
FAIL: nptl/tst-cancelx20
FAIL: nptl/tst-cancelx21
FAIL: nptl/tst-cond8-static
FAIL: nptl/tst-mutex8-static
FAIL: nptl/tst-mutexpi8-static
FAIL: nptl/tst-rwlock15

So it does look from my part.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32
  2024-01-15 14:38     ` Daniel Cederman
@ 2024-01-15 17:52       ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-15 17:52 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 15/01/24 11:38, Daniel Cederman wrote:
> On 2024-01-12 19:05, Adhemerval Zanella Netto wrote:
>>
>>
>> On 12/01/24 06:26, Daniel Cederman wrote:
>>> +/* Define these macros to 1 to workaround conversions of out-of-range
>>> +   floating-point numbers to integer types failing to raise the
>>> +   "invalid" exception, or raising spurious "inexact" or other
>>> +   exceptions.  */
>>
>> This is not really true for all sparc32, at least with sparcv9 I have
>> access I have not seen any issues with missing exception overflows or
>> spurious inexact.  Are these some leon3 limitation or issue?
>>
> 
> I don't think it is a leon specific issue. I can trigger the same issue with QEMU for other sparcs. Sparcv9 has custom versions of the functions involved so that is probably why it is not affected:
> 
> ./sysdeps/sparc/sparc32/sparcv9/fpu/multiarch/s_llrint.c
> ./sysdeps/sparc/sparc32/sparcv9/fpu/multiarch/s_llrintf.c
> 
> Corresponding patches for other 32-bit architectures references this bugzilla entry:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59412
> 
> I will have to look into this again, but I remember that it was a similar issue for sparc32.

You are right that sparcv9 has assembly specific implementation, but even
though by removing them and forcing libm to use the generic implementation
I am not seeing any regression.

I almost sure it is because on sparcv9, compile generates fdtox for double
to interger which is not subject to overflow issues.

> 
>> I think we need to know exactly how/when this happens and only enable
>> such workaound when required.
>>
>>> +#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
>>> +#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
>>> +#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
>>> +#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
>>> +
>>> +#endif /* fix-fp-int-convert-overflow.h */

So I think it should be something like:

#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
/* For sparcv9 compiler generates fdtox/fstox for floating point to
   integer generation.  */
#ifdef __sparcv9
# define FIX_FLT_LLONG_CONVERT_OVERFLOW 0
# define FIX_DBL_LLONG_CONVERT_OVERFLOW 0
#else
# define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
# define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
#endif

Even though it is not used currently on code generation.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Do not test preservation of NaN payloads for LEON
  2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
                   ` (4 preceding siblings ...)
  2024-01-12  9:26 ` [PATCH] sparc: Remove unwind information from signal return stub functions Daniel Cederman
@ 2024-01-16 15:37 ` Adhemerval Zanella Netto
  5 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-01-16 15:37 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas



On 12/01/24 06:26, Daniel Cederman wrote:
> The FPU used by LEON does not preserve NaN payload. This change allows
> the math/test-*-canonicalize tests to pass on LEON.
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  .../sparc32/fpu/math-tests-snan-payload.h     | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h
> 
> diff --git a/sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h b/sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h
> new file mode 100644
> index 0000000000..d84e4d997b
> --- /dev/null
> +++ b/sysdeps/sparc/sparc32/fpu/math-tests-snan-payload.h
> @@ -0,0 +1,30 @@
> +/* Configuration for math tests: sNaN payloads.  SPARC version.
> +   Copyright (C) 2016-2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef SPARC_MATH_TESTS_SNAN_PAYLOAD_H
> +#define SPARC_MATH_TESTS_SNAN_PAYLOAD_H 1
> +
> +/* LEON floating-point instructions do not preserve sNaN
> +   payloads.  */
> +#if defined (__leon__)
> +# define SNAN_TESTS_PRESERVE_PAYLOAD	0
> +#else
> +# define SNAN_TESTS_PRESERVE_PAYLOAD	1
> +#endif
> +
> +#endif /* math-tests-snan-payload.h.  */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Treat the version field in the FPU control word as reserved
  2024-01-12 17:42   ` Adhemerval Zanella Netto
@ 2024-02-15  9:31     ` Daniel Cederman
  2024-02-19 14:55       ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Cederman @ 2024-02-15  9:31 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha; +Cc: daniel, andreas

Hello Adhemerval,

Can you apply this to master if you are ok with the patch?

/Daniel

On 2024-01-12 18:42, Adhemerval Zanella Netto wrote:
> 
> 
> On 12/01/24 06:26, Daniel Cederman wrote:
>> The FSR version field is read-only and might be non-zero.
>>
>> This allows math/test-fpucw* to correctly pass when the version is
>> non-zero.
>>
>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> 
> It looks reasonable to mask off the version _FPU_RESERVED.  It also
> means that it would be change by __setfpucw, although afaik the ISA
> guarantee that it can not be changed anyway.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> 
>> ---
>>   sysdeps/sparc/fpu/fpu_control.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
>> index 9313743f86..36a2bf5d07 100644
>> --- a/sysdeps/sparc/fpu/fpu_control.h
>> +++ b/sysdeps/sparc/fpu/fpu_control.h
>> @@ -42,7 +42,7 @@
>>   #define _FPU_RC_ZERO    0x40000000
>>   #define _FPU_RC_NEAREST 0x0        /* RECOMMENDED */
>>   
>> -#define _FPU_RESERVED   0x30300000  /* Reserved bits in cw */
>> +#define _FPU_RESERVED   0x303e0000  /* Reserved bits in cw */
>>   
>>   
>>   /* Now two recommended cw */

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] sparc: Treat the version field in the FPU control word as reserved
  2024-02-15  9:31     ` Daniel Cederman
@ 2024-02-19 14:55       ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 21+ messages in thread
From: Adhemerval Zanella Netto @ 2024-02-19 14:55 UTC (permalink / raw)
  To: Daniel Cederman, libc-alpha; +Cc: daniel, andreas

It seems I forgot to push this one, it is done now.

On 15/02/24 06:31, Daniel Cederman wrote:
> Hello Adhemerval,
> 
> Can you apply this to master if you are ok with the patch?
> 
> /Daniel
> 
> On 2024-01-12 18:42, Adhemerval Zanella Netto wrote:
>>
>>
>> On 12/01/24 06:26, Daniel Cederman wrote:
>>> The FSR version field is read-only and might be non-zero.
>>>
>>> This allows math/test-fpucw* to correctly pass when the version is
>>> non-zero.
>>>
>>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
>>
>> It looks reasonable to mask off the version _FPU_RESERVED.  It also
>> means that it would be change by __setfpucw, although afaik the ISA
>> guarantee that it can not be changed anyway.
>>
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>>
>>> ---
>>>   sysdeps/sparc/fpu/fpu_control.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
>>> index 9313743f86..36a2bf5d07 100644
>>> --- a/sysdeps/sparc/fpu/fpu_control.h
>>> +++ b/sysdeps/sparc/fpu/fpu_control.h
>>> @@ -42,7 +42,7 @@
>>>   #define _FPU_RC_ZERO    0x40000000
>>>   #define _FPU_RC_NEAREST 0x0        /* RECOMMENDED */
>>>   -#define _FPU_RESERVED   0x30300000  /* Reserved bits in cw */
>>> +#define _FPU_RESERVED   0x303e0000  /* Reserved bits in cw */
>>>       /* Now two recommended cw */

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2024-02-19 14:55 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-12  9:26 [PATCH] sparc: Do not test preservation of NaN payloads for LEON Daniel Cederman
2024-01-12  9:26 ` [PATCH] sparc: Prevent stfsr from directly following floating-point instruction Daniel Cederman
2024-01-12 14:38   ` Adhemerval Zanella Netto
2024-01-15  9:37     ` Daniel Cederman
2024-01-12  9:26 ` [PATCH] sparc: Force calculation that raises exception Daniel Cederman
2024-01-12 16:45   ` Adhemerval Zanella Netto
2024-01-15  9:41     ` Daniel Cederman
2024-01-15 11:47       ` Adhemerval Zanella Netto
2024-01-12  9:26 ` [PATCH] sparc: Treat the version field in the FPU control word as reserved Daniel Cederman
2024-01-12 17:42   ` Adhemerval Zanella Netto
2024-02-15  9:31     ` Daniel Cederman
2024-02-19 14:55       ` Adhemerval Zanella Netto
2024-01-12  9:26 ` [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32 Daniel Cederman
2024-01-12 18:05   ` Adhemerval Zanella Netto
2024-01-15 14:38     ` Daniel Cederman
2024-01-15 17:52       ` Adhemerval Zanella Netto
2024-01-12  9:26 ` [PATCH] sparc: Remove unwind information from signal return stub functions Daniel Cederman
2024-01-15 13:41   ` Adhemerval Zanella Netto
2024-01-15 14:22     ` Daniel Cederman
2024-01-15 16:57       ` Adhemerval Zanella Netto
2024-01-16 15:37 ` [PATCH] sparc: Do not test preservation of NaN payloads for LEON Adhemerval Zanella Netto

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).