sparc: Prevent stfsr from directly following floating-point instruction

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

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
>
  

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