[PATCH v2 1/3] math: Remove fenvinline.h

[Paul E Murphy]

On 3/10/20 11:51 AM, Adhemerval Zanella wrote:
> 
> 
> On 09/03/2020 17:19, Paul E Murphy wrote:
>>
>>
>> On 3/9/20 1:32 PM, Adhemerval Zanella wrote:
>>> Changes from previous version:
>>>
>>>     - Mention on commit message x86 also exports a similar optimization,
>>>       but on a different header.
>>>
>>> -- 
>>>
>>> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
>>> patch removes the fenvinline.h on all architectures.  Currently
>>> only powerpc implements some optimizations.  This kind of optimization
>>> is better implemented by the compiler (which handles the architecture
>>> ISA transparently).
>>>
>>> Also, for the specific optimized powerpc implementation the code is
>>> becoming convoluted and these micro-optimization are hardly wildly
>>> used, even more being a possible hotspot in realword cases
>>> (non-default rounding are used only on specific cases and exception
>>> handling are done most likely only on errors path).  Only x86
>>> implements similar optimization (on fenv.h) also indicates that
>>> these should no be on libc.
>>>
>>> The math/test-fenv already covers all math/test-fenvinline tests,
>>> so it is safe to remove it.
>>>
>>> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
>>> ---
>>>    bits/fenvinline.h                 |   8 -
>>>    math/Makefile                     |   4 +-
>>>    math/fenv.h                       |   4 -
>>>    math/test-fenvinline.c            | 354 ------------------------------
>>>    sysdeps/powerpc/bits/fenvinline.h | 108 ---------
>>>    5 files changed, 2 insertions(+), 476 deletions(-)
>>>    delete mode 100644 bits/fenvinline.h
>>>    delete mode 100644 math/test-fenvinline.c
>>>    delete mode 100644 sysdeps/powerpc/bits/fenvinline.h
> 
> Indeed, I misread the failures on powerpc64le-linux-gnu.  Below it is
> an updated patch with the fegetround optimization moved to an internal
> header.

> diff --git a/sysdeps/powerpc/fpu/fegetround.c b/sysdeps/powerpc/fpu/fegetround.c
> index 00b4462624..9d7762f08b 100644
> --- a/sysdeps/powerpc/fpu/fegetround.c
> +++ b/sysdeps/powerpc/fpu/fegetround.c
> @@ -21,10 +21,8 @@
>   int
>   (__fegetround) (void)
>   {
> -  return __fegetround();
> +  return __fegetround_inline ();
>   }
> -#undef fegetround
> -#undef __fegetround
>   libm_hidden_def (__fegetround)
>   weak_alias (__fegetround, fegetround)
>   libm_hidden_weak (fegetround)
> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
> index e888c6621c..09dbd3e2df 100644
> --- a/sysdeps/powerpc/fpu/fenv_libc.h
> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
> @@ -68,6 +68,14 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
>       __fr;								\
>     })
>   
> +#define __fe_mffsl()							\
> +  ({register fenv_union_t __fr;						\
> +    __asm__ __volatile__ (						\
> +      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
> +      : "=f" (__fr.fenv));						\
> +    __fr.l & 0x3;							\
> +  })
> + >   #define __fe_mffscrn(rn)						\
>     ({register fenv_union_t __fr;						\
>       if (__builtin_constant_p (rn))					\
> @@ -144,6 +152,20 @@ typedef union
>     unsigned long long l;
>   } fenv_union_t;
>   
> +static inline int
> +__fegetround_inline (void)
> +{
> +#ifdef _ARCH_PWR9
> +  return __fe_mffsl ();
> +#else
> +  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
> +    return __fe_mffsl ();
> +
Can the above be removed, and fegetenv_register() be replaced with 
fegetenv_control()?  Such should work optimally on all ppc machines. 
Otherwise, it LGTM.

I have mixed feelings about regressing these inlines before compiler 
support arrives, but I suspect these are likely not used in performance 
critical places, so I am not objecting.

> +  fenv_union_t fe;
> +  fe.fenv = fegetenv_register ();
> +  return fe.l & 0x3;
> +#endif
> +}
>   
>   static inline int
>   __fesetround_inline (int round)