[PATCH v2 2/4] iee754: provide gcc builtins based generic fma functions

[Adhemerval Zanella]

On 02/06/2020 14:13, Vineet Gupta via Libc-alpha wrote:
> On 6/2/20 5:51 AM, Stefan Liebler via Libc-alpha wrote:
>>  #endif /* math-use-builtins.h */
>> Please also update the current architecture specific math-use-builtins.h
>> file: sysdeps/s390/fpu/math-use-builtins.h
>> Otherwise it will break build on s390x.
> 
> Done.
> 
>>> diff --git a/sysdeps/ieee754/dbl-64/s_fma.c b/sysdeps/ieee754/dbl-64/s_fma.c
>>> index 876df6e78bdc..9dc5b132b9ee 100644
>>> --- a/sysdeps/ieee754/dbl-64/s_fma.c
>>> +++ b/sysdeps/ieee754/dbl-64/s_fma.c
>>> @@ -25,6 +25,7 @@
>>>  #include <fenv_private.h>
>>>  #include <libm-alias-double.h>
>>>  #include <tininess.h>
>>> +#include <math-use-builtins.h>
>>>
>>>  /* This implementation uses rounding to odd to avoid problems with
>>>     double rounding.  See a paper by Boldo and Melquiond:
>>> @@ -33,6 +34,10 @@
>>>  double
>>>  __fma (double x, double y, double z)
>>>  {
>>> +#if USE_FMA_BUILTIN
>>> +  return __builtin_fma (x, y, z);
>>
>> Architectures which have support for ldbl-128 will use the file
>> sysdeps/ieee754/ldbl-128/s_fma.c instead of
>> sysdeps/ieee754/dbl-64/s_fma.c. Should this file also be adjusted in
>> order to use the builtin if USE_FMA_BUILTIN is set to one?
> 
> Right.
> 
> I used commit f82996f815 "Use GCC builtins for round functions if desired" as
> starting point for my change. And seems it was not an ideal reference :-) as round
> has far fewer instances than fma. Indeed fma is present in ldbl-128 and dbl-64 so
> needs updating in both.

I think after this set is upstream I will refactor to make each symbol and its
variant (i.g sqrt, sqrtf, etc.) to be define on its own file.  The default
math-use-builtins will then include each file:

  /* math-use-builtins.h  */
  #inclde <math-use-builtins-sqrt.h>
  [...]

With default USE_* begin 0. This would allow to add new builtin usage without 
require to sync with every architecture (and without breaking its build as well).

> 
> But just to be sure s390 is currently not using the newly introduced builtins so
> I'll keep them as follows.
> 
> #define USE_SQRT_BUILTIN 0
> #define USE_SQRTF_BUILTIN 0
> 
> #define USE_FMA_BUILTIN 0
> #define USE_FMAF_BUILTIN 0
> #define USE_FMAL_BUILTIN 0
> #define USE_FMAF128_BUILTIN 0
> 
> -Vineet

It is ok, we can optimize s390 in a subsequent patch (since it does provide fma
and sqrtf builtins).