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

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

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
>