powerpc: define USE_FMAF128_BUILTIN in math-use-builtins.h

Message ID 20200603200756.1495911-1-murphyp@linux.vnet.ibm.com
State Superseded
Headers
Series powerpc: define USE_FMAF128_BUILTIN in math-use-builtins.h |

Commit Message

Paul E. Murphy June 3, 2020, 8:07 p.m. UTC
  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       |  4 +++
 .../le/fpu/multiarch/s_fmaf128-power9.c       |  4 ++-
 .../powerpc64/le/power9/fpu/s_fmaf128.c       | 36 -------------------
 3 files changed, 7 insertions(+), 37 deletions(-)
 delete mode 100644 sysdeps/powerpc/powerpc64/le/power9/fpu/s_fmaf128.c
  

Comments

Andreas Schwab June 3, 2020, 8:43 p.m. UTC | #1
On Jun 03 2020, Paul E. Murphy via Libc-alpha wrote:

> diff --git a/sysdeps/powerpc/fpu/math-use-builtins.h b/sysdeps/powerpc/fpu/math-use-builtins.h
> index 4780934379..e4b9b18659 100644
> --- a/sysdeps/powerpc/fpu/math-use-builtins.h
> +++ b/sysdeps/powerpc/fpu/math-use-builtins.h
> @@ -66,4 +66,8 @@
>  #define USE_FMA_BUILTIN 1
>  #define USE_FMAF_BUILTIN 1
>  
> +/*  This is not available for P8 or BE targets.  */
> +#define USE_FMAF128_BUILTIN (defined(__FP_FAST_FMAF128) \
> +                            && __FP_FAST_FMAF128 == 1)

Using defined in a macro expansion is undefined.

Andreas.
  
Adhemerval Zanella June 4, 2020, 11:39 a.m. UTC | #2
On 03/06/2020 17:07, Paul E. Murphy via Libc-alpha wrote:
> 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.

I should have spotted that testing only powerpc-linux-gnu for the
patch [1] was not suffice.

[1] https://sourceware.org/pipermail/libc-alpha/2020-June/114565.html

> ---
>  sysdeps/powerpc/fpu/math-use-builtins.h       |  4 +++
>  .../le/fpu/multiarch/s_fmaf128-power9.c       |  4 ++-
>  .../powerpc64/le/power9/fpu/s_fmaf128.c       | 36 -------------------
>  3 files changed, 7 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..e4b9b18659 100644
> --- a/sysdeps/powerpc/fpu/math-use-builtins.h
> +++ b/sysdeps/powerpc/fpu/math-use-builtins.h
> @@ -66,4 +66,8 @@
>  #define USE_FMA_BUILTIN 1
>  #define USE_FMAF_BUILTIN 1
>  
> +/*  This is not available for P8 or BE targets.  */
> +#define USE_FMAF128_BUILTIN (defined(__FP_FAST_FMAF128) \
> +                            && __FP_FAST_FMAF128 == 1)
> +
>  #endif

I think it should be:

#ifdef __FP_FAST_FMAF128
# define USE_FMAF128_BUILTIN 1
#else
# define USE_FMAF128_BUILTIN 0
#endif

I don't think you need to check for define value, afaik gcc will
always define it to 1 if you are targeting a architecture that
supports it.

However I will push a quick fix to enable powerpc64le (with
USE_FMAF128_BUILTIN set to 0) and and we can enable fmaf128 in a 
subsequent patch (since it requires more fixes for ifunc).

> 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)
>
  

Patch

diff --git a/sysdeps/powerpc/fpu/math-use-builtins.h b/sysdeps/powerpc/fpu/math-use-builtins.h
index 4780934379..e4b9b18659 100644
--- a/sysdeps/powerpc/fpu/math-use-builtins.h
+++ b/sysdeps/powerpc/fpu/math-use-builtins.h
@@ -66,4 +66,8 @@ 
 #define USE_FMA_BUILTIN 1
 #define USE_FMAF_BUILTIN 1
 
+/*  This is not available for P8 or BE targets.  */
+#define USE_FMAF128_BUILTIN (defined(__FP_FAST_FMAF128) \
+                            && __FP_FAST_FMAF128 == 1)
+
 #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)