[PATCH v2] sparc: Force calculation that raises exception

[PATCH v2] sparc: Force calculation that raises exception

[Daniel Cederman]

Use the math_force_eval() macro to force the calculation to complete and
raise the exception.

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

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

diff --git a/sysdeps/sparc/fpu/fraiseexcpt.c b/sysdeps/sparc/fpu/fraiseexcpt.c
index 26a7720ec9..87c8f9a4dc 100644
--- a/sysdeps/sparc/fpu/fraiseexcpt.c
+++ b/sysdeps/sparc/fpu/fraiseexcpt.c
@@ -42,7 +42,7 @@ __feraiseexcept (int excepts)
       /* One example of an invalid operation is 0/0.  */
       __asm ("" : "=e" (d) : "0" (c.zero));
       d /= c.zero;
-      __asm __volatile ("" : : "e" (d));
+      math_force_eval (d);
     }
 
   /* Next: division by zero.  */
@@ -50,7 +50,7 @@ __feraiseexcept (int excepts)
     {
       __asm ("" : "=e" (d) : "0" (c.one));
       d /= c.zero;
-      __asm __volatile ("" : : "e" (d));
+      math_force_eval (d);
     }
 
   /* Next: overflow.  */
@@ -58,7 +58,7 @@ __feraiseexcept (int excepts)
     {
       __asm ("" : "=e" (d) : "0" (c.max));
       d *= d;
-      __asm __volatile ("" : : "e" (d));
+      math_force_eval (d);
     }
 
   /* Next: underflow.  */
@@ -66,7 +66,7 @@ __feraiseexcept (int excepts)
     {
       __asm ("" : "=e" (d) : "0" (c.min));
       d *= d;
-      __asm __volatile ("" : : "e" (d));
+      math_force_eval (d);
     }
 
   /* Last: inexact.  */
@@ -74,7 +74,7 @@ __feraiseexcept (int excepts)
     {
       __asm ("" : "=e" (d) : "0" (c.one));
       d /= c.pi;
-      __asm __volatile ("" : : "e" (d));
+      math_force_eval (d);
     }
 
   /* Success.  */
-- 
2.40.1

[PATCH v2 1/2] sparc: Use existing macros to avoid code duplication

[Daniel Cederman]

Macros for using inline assembly to access the fp state register exists
in both fenv_private.h and in fpu_control.h. Let fenv_private.h use the
macros from fpu_control.h

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

diff --git a/sysdeps/sparc/fpu/fenv_private.h b/sysdeps/sparc/fpu/fenv_private.h
index da7c7fe332..a31ba3cc7b 100644
--- a/sysdeps/sparc/fpu/fenv_private.h
+++ b/sysdeps/sparc/fpu/fenv_private.h
@@ -2,15 +2,11 @@
 #define SPARC_FENV_PRIVATE_H 1
 
 #include <fenv.h>
+#include <fpu_control.h>
 
 /* For internal use only: access the fp state register.  */
-#if __WORDSIZE == 64
-# 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))
-# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
-#endif
+#define __fenv_stfsr(X)   _FPU_GETCW (X)
+#define __fenv_ldfsr(X)   _FPU_SETCW (X)
 
 static __always_inline void
 libc_feholdexcept (fenv_t *e)
-- 
2.40.1

[PATCH v2 2/2] sparc: Prevent stfsr from directly following floating-point instruction

[Daniel Cederman]

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/fpu_control.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
index dd18789573..48368a7ce1 100644
--- a/sysdeps/sparc/fpu/fpu_control.h
+++ b/sysdeps/sparc/fpu/fpu_control.h
@@ -61,7 +61,12 @@ 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__
+   /* Prevent stfsr from being placed directly after other fp instruction.  */
+#  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

Re: [PATCH v2] sparc: Force calculation that raises exception

[Adhemerval Zanella Netto]

On 15/01/24 11:53, Daniel Cederman wrote:
> Use the math_force_eval() macro to force the calculation to complete and
> raise the exception.
> 
> With this change the math/test-fenv test pass.
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> ---
>  sysdeps/sparc/fpu/fraiseexcpt.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/sysdeps/sparc/fpu/fraiseexcpt.c b/sysdeps/sparc/fpu/fraiseexcpt.c
> index 26a7720ec9..87c8f9a4dc 100644
> --- a/sysdeps/sparc/fpu/fraiseexcpt.c
> +++ b/sysdeps/sparc/fpu/fraiseexcpt.c

You need to include '<math-barriers.h>' (I am getting building failure
without it).

> @@ -42,7 +42,7 @@ __feraiseexcept (int excepts)
>        /* One example of an invalid operation is 0/0.  */
>        __asm ("" : "=e" (d) : "0" (c.zero));
>        d /= c.zero;
> -      __asm __volatile ("" : : "e" (d));
> +      math_force_eval (d);
>      }
>  
>    /* Next: division by zero.  */
> @@ -50,7 +50,7 @@ __feraiseexcept (int excepts)
>      {
>        __asm ("" : "=e" (d) : "0" (c.one));
>        d /= c.zero;
> -      __asm __volatile ("" : : "e" (d));
> +      math_force_eval (d);
>      }
>  
>    /* Next: overflow.  */
> @@ -58,7 +58,7 @@ __feraiseexcept (int excepts)
>      {
>        __asm ("" : "=e" (d) : "0" (c.max));
>        d *= d;
> -      __asm __volatile ("" : : "e" (d));
> +      math_force_eval (d);
>      }
>  
>    /* Next: underflow.  */
> @@ -66,7 +66,7 @@ __feraiseexcept (int excepts)
>      {
>        __asm ("" : "=e" (d) : "0" (c.min));
>        d *= d;
> -      __asm __volatile ("" : : "e" (d));
> +      math_force_eval (d);
>      }
>  
>    /* Last: inexact.  */
> @@ -74,7 +74,7 @@ __feraiseexcept (int excepts)
>      {
>        __asm ("" : "=e" (d) : "0" (c.one));
>        d /= c.pi;
> -      __asm __volatile ("" : : "e" (d));
> +      math_force_eval (d);
>      }
>  
>    /* Success.  */

Re: [PATCH v2 1/2] sparc: Use existing macros to avoid code duplication

[Adhemerval Zanella Netto]

On 15/01/24 11:53, Daniel Cederman wrote:
> Macros for using inline assembly to access the fp state register exists
> in both fenv_private.h and in fpu_control.h. Let fenv_private.h use the
> macros from fpu_control.h
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>

LGTM, thanks.

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

> ---
>  sysdeps/sparc/fpu/fenv_private.h | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/sysdeps/sparc/fpu/fenv_private.h b/sysdeps/sparc/fpu/fenv_private.h
> index da7c7fe332..a31ba3cc7b 100644
> --- a/sysdeps/sparc/fpu/fenv_private.h
> +++ b/sysdeps/sparc/fpu/fenv_private.h
> @@ -2,15 +2,11 @@
>  #define SPARC_FENV_PRIVATE_H 1
>  
>  #include <fenv.h>
> +#include <fpu_control.h>
>  
>  /* For internal use only: access the fp state register.  */
> -#if __WORDSIZE == 64
> -# 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))
> -# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
> -#endif
> +#define __fenv_stfsr(X)   _FPU_GETCW (X)
> +#define __fenv_ldfsr(X)   _FPU_SETCW (X)
>  
>  static __always_inline void
>  libc_feholdexcept (fenv_t *e)

Re: [PATCH v2 2/2] sparc: Prevent stfsr from directly following floating-point instruction

[Adhemerval Zanella Netto]

On 15/01/24 11:53, 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>

LGTM, thanks.

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

> ---
>  sysdeps/sparc/fpu/fpu_control.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/sparc/fpu/fpu_control.h b/sysdeps/sparc/fpu/fpu_control.h
> index dd18789573..48368a7ce1 100644
> --- a/sysdeps/sparc/fpu/fpu_control.h
> +++ b/sysdeps/sparc/fpu/fpu_control.h
> @@ -61,7 +61,12 @@ 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__
> +   /* Prevent stfsr from being placed directly after other fp instruction.  */
> +#  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
>  

Re: [PATCH v2] sparc: Force calculation that raises exception

[Daniel Cederman]

On 2024-01-16 14:57, Adhemerval Zanella Netto wrote:
> 
> 
> On 15/01/24 11:53, Daniel Cederman wrote:
>> Use the math_force_eval() macro to force the calculation to complete and
>> raise the exception.
>>
>> With this change the math/test-fenv test pass.
>>
>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
>> ---
>>   sysdeps/sparc/fpu/fraiseexcpt.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/sysdeps/sparc/fpu/fraiseexcpt.c b/sysdeps/sparc/fpu/fraiseexcpt.c
>> index 26a7720ec9..87c8f9a4dc 100644
>> --- a/sysdeps/sparc/fpu/fraiseexcpt.c
>> +++ b/sysdeps/sparc/fpu/fraiseexcpt.c
> 
> You need to include '<math-barriers.h>' (I am getting building failure
> without it).
> 

Sorry about that, I will post an updated version with this fixed.

Thank you for reviewing the patches! Can I ask you to also take a look
at the "sparc: Do not test preservation of NaN payloads for LEON" patch?
The FPU used by LEON does not support propagating NaN payloads 
(documented in Section 51.2.7 in 
https://www.gaisler.com/products/grlib/grip.pdf)