Re: [PATCH] [powerpc] fegetenv_and_set_rn now uses the builtins provided by GCC.

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

On 12/09/23 05:27, Manjunath Matti wrote:
> On powerpc, SET_RESTORE_ROUND uses inline assembly to optimize the
> prologue get/save/set rounding mode operations for POWER9 and
> later by using 'mffscrn' where possible, this was introduced by
> commit f1c56cdff09f650ad721fae026eb6a3651631f3d.
> 
> GCC version 14 onwards supports builtins as __builtin_set_fpscr_rn
> which now returns the FPSCR fields in a double. This feature is
> available on Power9 when the __SET_FPSCR_RN_RETURNS_FPSCR__ macro
> is defined along with __builtin_set_fpscr_rn enabled.
> GCC commit ef3bbc69d15707e4db6e2f198c621effb636cc26 adds
> this feature.
> 
> Changes are done to use __builtin_set_fpscr_rn instead of mffscrn
> or mffscrni in __fe_mffscrn(rn).
> 
> Suggested-by: Carl Love <cel@us.ibm.com>
> ---
>  sysdeps/powerpc/fpu/fenv_libc.h | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
> index fa5e1c697e..55484eb229 100644
> --- a/sysdeps/powerpc/fpu/fenv_libc.h
> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
> @@ -84,8 +84,15 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
>      __fr.fenv;								\
>    })
>  
> +/* GCC version 14 onwards supports builtins as __builtin_set_fpscr_rn and
> +   now returns the FPSCR fields in a double. This support is available
> +   on Power9 when the __SET_FPSCR_RN_RETURNS_FPSCR__ macro is defined.
> +   To retain backward compatibility with older GCC, we still retain the
> +   old inline assembly implementation.  */
> +#if defined _ARCH_PWR9 && defined __SET_FPSCR_RN_RETURNS_FPSCR__
> +#define fegetenv_and_set_rn(rn) __builtin_set_fpscr_rn (rn)
> +#elif defined _ARCH_PWR9
>  /* Like fegetenv_control, but also sets the rounding mode.  */
> -#ifdef _ARCH_PWR9
>  #define fegetenv_and_set_rn(rn) __fe_mffscrn (rn)
>  #else
>  /* 'mffscrn' will decode to 'mffs' on ARCH < 3_00, which is still necessary

I think the macro would be better defined as:

#ifdef __SET_FPSCR_RN_RETURNS_FPSCR__
# define __fe_mffscrn(rn)  __builtin_set_fpscr_rn (rn)
#else
# define __fe_mffscrn(rn) [...]
#endif

Then there is no need to redefine fegetenv_and_set_rn nor __fesetround_inline.

> @@ -148,7 +155,12 @@ typedef union
>  static inline int
>  __fesetround_inline (int round)
>  {
> -#ifdef _ARCH_PWR9
> +/* GCC version 14 onwards supports builtins as __builtin_set_fpscr_rn and
> +   now returns the FPSCR fields in a double. This support is available
> +   on Power9 when the __SET_FPSCR_RN_RETURNS_FPSCR__ macro is defined.  */
> +#if defined _ARCH_PWR9 && defined __SET_FPSCR_RN_RETURNS_FPSCR__
> +  __builtin_set_fpscr_rn (round);
> +#elif defined _ARCH_PWR9
>    __fe_mffscrn (round);
>  #else
>    if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
> @@ -178,7 +190,12 @@ __fesetround_inline (int round)
>  static inline void
>  __fesetround_inline_nocheck (const int round)
>  {
> -#ifdef _ARCH_PWR9
> +/* GCC version 14 onwards supports builtins as __builtin_set_fpscr_rn and
> +   now returns the FPSCR fields in a double. This support is available
> +   on Power9 when the __SET_FPSCR_RN_RETURNS_FPSCR__ macro is defined.  */
> +#if defined _ARCH_PWR9 && defined __SET_FPSCR_RN_RETURNS_FPSCR__
> +  __builtin_set_fpscr_rn (round);
> +#elif defined _ARCH_PWR9
>    __fe_mffscrn (round);
>  #else
>    if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))