[PATCHv2] powerpc: define USE_FMAF128_BUILTIN in math-use-builtins.h

Message ID 20200604135656.2121535-1-murphyp@linux.vnet.ibm.com
State Superseded
Delegated to: Adhemerval Zanella Netto
Headers
Series [PATCHv2] powerpc: define USE_FMAF128_BUILTIN in math-use-builtins.h |

Commit Message

Paul E. Murphy June 4, 2020, 1:56 p.m. UTC
  Removed defined in macro expansion.  Verified again.  Thank you for
pointing out the macro expansion nuance of defined Andreas.

---8<---

Commit a7a3435c9a0769744c7748f9d95510d0a99be7d1 caused an undefined
macro evaluation error when building powerpc64le.  This defines the
macro such that it should behave correctly on all supported powerpc64le
targets.  Likewise, this allows us to remove the ppc64le specific
s_fmaf128.c for power9.

powerpc{,64} will not support FMA using the ibm128 long double, so
it can be left undefined for now to avoid ambiguity with ppc64le.

I have verified powerpc64le multiarch and powerpc64le power9
no-multiarch builds continue to generate optimize fmaf128.
---
 sysdeps/powerpc/fpu/math-use-builtins.h       |  7 ++++
 .../le/fpu/multiarch/s_fmaf128-power9.c       |  4 ++-
 .../powerpc64/le/power9/fpu/s_fmaf128.c       | 36 -------------------
 3 files changed, 10 insertions(+), 37 deletions(-)
 delete mode 100644 sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
  

Comments

Adhemerval Zanella Netto June 4, 2020, 2:03 p.m. UTC | #1
On 04/06/2020 10:56, Paul E. Murphy via Libc-alpha wrote:
> Removed defined in macro expansion.  Verified again.  Thank you for
> pointing out the macro expansion nuance of defined Andreas.
> 
> ---8<---
> 
> Commit a7a3435c9a0769744c7748f9d95510d0a99be7d1 caused an undefined
> macro evaluation error when building powerpc64le.  This defines the
> macro such that it should behave correctly on all supported powerpc64le
> targets.  Likewise, this allows us to remove the ppc64le specific
> s_fmaf128.c for power9.
> 
> powerpc{,64} will not support FMA using the ibm128 long double, so
> it can be left undefined for now to avoid ambiguity with ppc64le.
> 
> I have verified powerpc64le multiarch and powerpc64le power9
> no-multiarch builds continue to generate optimize fmaf128.
> ---
>  sysdeps/powerpc/fpu/math-use-builtins.h       |  7 ++++
>  .../le/fpu/multiarch/s_fmaf128-power9.c       |  4 ++-
>  .../powerpc64/le/power9/fpu/s_fmaf128.c       | 36 -------------------
>  3 files changed, 10 insertions(+), 37 deletions(-)
>  delete mode 100644 sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
> 
> diff --git a/sysdeps/powerpc/fpu/math-use-builtins.h b/sysdeps/powerpc/fpu/math-use-builtins.h
> index 4780934379..9b1cdfc4c8 100644
> --- a/sysdeps/powerpc/fpu/math-use-builtins.h
> +++ b/sysdeps/powerpc/fpu/math-use-builtins.h
> @@ -66,4 +66,11 @@
>  #define USE_FMA_BUILTIN 1
>  #define USE_FMAF_BUILTIN 1
>  
> +/*  This is not available for P8 or BE targets.  */
> +#if defined (__FP_FAST_FMAF128) && __FP_FAST_FMAF128 == 1
> +# define USE_FMAF128_BUILTIN 1
> +#else
> +# define USE_FMAF128_BUILTIN 0
> +#endif

Why do you need to test for __FP_FAST_FMAF128 value? My understand
from gcc documentation is it will be set iff the backend supports
lowering the operation. Is there configuration where gcc might
define and sets its value to 0?

> +
>  #endif
> diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c
> index 8df77ceade..49aeb3a8f4 100644
> --- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c
> +++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c
> @@ -20,7 +20,9 @@
>  
>  #undef libm_alias_float128
>  #define libm_alias_float128(a, b)
> +#undef strong_alias
> +#define strong_alias(a, b)
>  
>  #define __fmaf128 __fmaf128_power9
>  
> -#include <sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c>
> +#include <sysdeps/ieee754/float128/s_fmaf128.c>
> diff --git a/sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c b/sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
> deleted file mode 100644
> index f02e810fb9..0000000000
> --- a/sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -/* Compute x * y + z as a ternary operation for _Float128.  POWER9 version.
> -   Copyright (C) 2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   In addition to the permissions in the GNU Lesser General Public
> -   License, the Free Software Foundation gives you unlimited
> -   permission to link the compiled version of this file into
> -   combinations with other programs, and to distribute those
> -   combinations without any restriction coming from the use of this
> -   file.  (The Lesser General Public License restrictions do apply in
> -   other respects; for example, they cover modification of the file,
> -   and distribution when not linked into a combine executable.)
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <libm-alias-float128.h>
> -
> -__float128
> -__fmaf128 (__float128 x, __float128 y, __float128 z)
> -{
> -  return x * y + z;
> -}
> -
> -libm_alias_float128 (__fma, fma)
>
  
Paul E Murphy June 4, 2020, 4:19 p.m. UTC | #2
On 6/4/20 9:03 AM, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 04/06/2020 10:56, Paul E. Murphy via Libc-alpha wrote:
>> Removed defined in macro expansion.  Verified again.  Thank you for
>> pointing out the macro expansion nuance of defined Andreas.
>>
>> ---8<---
>>
>> Commit a7a3435c9a0769744c7748f9d95510d0a99be7d1 caused an undefined
>> macro evaluation error when building powerpc64le.  This defines the
>> macro such that it should behave correctly on all supported powerpc64le
>> targets.  Likewise, this allows us to remove the ppc64le specific
>> s_fmaf128.c for power9.
>>
>> powerpc{,64} will not support FMA using the ibm128 long double, so
>> it can be left undefined for now to avoid ambiguity with ppc64le.
>>
>> I have verified powerpc64le multiarch and powerpc64le power9
>> no-multiarch builds continue to generate optimize fmaf128.
>> ---
>>   sysdeps/powerpc/fpu/math-use-builtins.h       |  7 ++++
>>   .../le/fpu/multiarch/s_fmaf128-power9.c       |  4 ++-
>>   .../powerpc64/le/power9/fpu/s_fmaf128.c       | 36 -------------------
>>   3 files changed, 10 insertions(+), 37 deletions(-)
>>   delete mode 100644 sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
>>
>> diff --git a/sysdeps/powerpc/fpu/math-use-builtins.h b/sysdeps/powerpc/fpu/math-use-builtins.h
>> index 4780934379..9b1cdfc4c8 100644
>> --- a/sysdeps/powerpc/fpu/math-use-builtins.h
>> +++ b/sysdeps/powerpc/fpu/math-use-builtins.h
>> @@ -66,4 +66,11 @@
>>   #define USE_FMA_BUILTIN 1
>>   #define USE_FMAF_BUILTIN 1
>>   
>> +/*  This is not available for P8 or BE targets.  */
>> +#if defined (__FP_FAST_FMAF128) && __FP_FAST_FMAF128 == 1
>> +# define USE_FMAF128_BUILTIN 1
>> +#else
>> +# define USE_FMAF128_BUILTIN 0
>> +#endif
> 
> Why do you need to test for __FP_FAST_FMAF128 value? My understand
> from gcc documentation is it will be set iff the backend supports
> lowering the operation. Is there configuration where gcc might
> define and sets its value to 0?

Maybe not, this may be needlessly pedantic.  Anyhow, I think you've 
mitigated the build error.  Attached is a V3.
  
Paul E Murphy June 5, 2020, 8:43 p.m. UTC | #3
On 6/4/20 11:19 AM, Paul E Murphy via Libc-alpha wrote:
> 
> 
> On 6/4/20 9:03 AM, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 04/06/2020 10:56, Paul E. Murphy via Libc-alpha wrote:
>>> Removed defined in macro expansion.  Verified again.  Thank you for
>>> pointing out the macro expansion nuance of defined Andreas.
>>>
>>> ---8<---
>>>
>>> Commit a7a3435c9a0769744c7748f9d95510d0a99be7d1 caused an undefined
>>> macro evaluation error when building powerpc64le.  This defines the
>>> macro such that it should behave correctly on all supported powerpc64le
>>> targets.  Likewise, this allows us to remove the ppc64le specific
>>> s_fmaf128.c for power9.
>>>
>>> powerpc{,64} will not support FMA using the ibm128 long double, so
>>> it can be left undefined for now to avoid ambiguity with ppc64le.
>>>
>>> I have verified powerpc64le multiarch and powerpc64le power9
>>> no-multiarch builds continue to generate optimize fmaf128.
>>> ---
>>>   sysdeps/powerpc/fpu/math-use-builtins.h       |  7 ++++
>>>   .../le/fpu/multiarch/s_fmaf128-power9.c       |  4 ++-
>>>   .../powerpc64/le/power9/fpu/s_fmaf128.c       | 36 -------------------
>>>   3 files changed, 10 insertions(+), 37 deletions(-)
>>>   delete mode 100644 sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
>>>
>>> diff --git a/sysdeps/powerpc/fpu/math-use-builtins.h 
>>> b/sysdeps/powerpc/fpu/math-use-builtins.h
>>> index 4780934379..9b1cdfc4c8 100644
>>> --- a/sysdeps/powerpc/fpu/math-use-builtins.h
>>> +++ b/sysdeps/powerpc/fpu/math-use-builtins.h
>>> @@ -66,4 +66,11 @@
>>>   #define USE_FMA_BUILTIN 1
>>>   #define USE_FMAF_BUILTIN 1
>>> +/*  This is not available for P8 or BE targets.  */
>>> +#if defined (__FP_FAST_FMAF128) && __FP_FAST_FMAF128 == 1
>>> +# define USE_FMAF128_BUILTIN 1
>>> +#else
>>> +# define USE_FMAF128_BUILTIN 0
>>> +#endif
>>
>> Why do you need to test for __FP_FAST_FMAF128 value? My understand
>> from gcc documentation is it will be set iff the backend supports
>> lowering the operation. Is there configuration where gcc might
>> define and sets its value to 0?
> 
> Maybe not, this may be needlessly pedantic.  Anyhow, I think you've 
> mitigated the build error.  Attached is a V3.

And V3 is pushed with a minor typo cleanup in the commit message.
  

Patch

diff --git a/sysdeps/powerpc/fpu/math-use-builtins.h b/sysdeps/powerpc/fpu/math-use-builtins.h
index 4780934379..9b1cdfc4c8 100644
--- a/sysdeps/powerpc/fpu/math-use-builtins.h
+++ b/sysdeps/powerpc/fpu/math-use-builtins.h
@@ -66,4 +66,11 @@ 
 #define USE_FMA_BUILTIN 1
 #define USE_FMAF_BUILTIN 1
 
+/*  This is not available for P8 or BE targets.  */
+#if defined (__FP_FAST_FMAF128) && __FP_FAST_FMAF128 == 1
+# define USE_FMAF128_BUILTIN 1
+#else
+# define USE_FMAF128_BUILTIN 0
+#endif
+
 #endif
diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c
index 8df77ceade..49aeb3a8f4 100644
--- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c
+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/s_fmaf128-power9.c
@@ -20,7 +20,9 @@ 
 
 #undef libm_alias_float128
 #define libm_alias_float128(a, b)
+#undef strong_alias
+#define strong_alias(a, b)
 
 #define __fmaf128 __fmaf128_power9
 
-#include <sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c>
+#include <sysdeps/ieee754/float128/s_fmaf128.c>
diff --git a/sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c b/sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
deleted file mode 100644
index f02e810fb9..0000000000
--- a/sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
+++ /dev/null
@@ -1,36 +0,0 @@ 
-/* Compute x * y + z as a ternary operation for _Float128.  POWER9 version.
-   Copyright (C) 2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   In addition to the permissions in the GNU Lesser General Public
-   License, the Free Software Foundation gives you unlimited
-   permission to link the compiled version of this file into
-   combinations with other programs, and to distribute those
-   combinations without any restriction coming from the use of this
-   file.  (The Lesser General Public License restrictions do apply in
-   other respects; for example, they cover modification of the file,
-   and distribution when not linked into a combine executable.)
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <libm-alias-float128.h>
-
-__float128
-__fmaf128 (__float128 x, __float128 y, __float128 z)
-{
-  return x * y + z;
-}
-
-libm_alias_float128 (__fma, fma)