This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] powerpc: Add multiarch sqrtf128 for ppc64le
- From: "Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>
- To: Rajalakshmi Srinivasaraghavan <raji at linux dot vnet dot ibm dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Tue, 29 May 2018 12:37:58 -0300
- Subject: Re: [PATCH] powerpc: Add multiarch sqrtf128 for ppc64le
- References: <1526532912-1716-1-git-send-email-raji@linux.vnet.ibm.com>
On Thu, 17 May 2018, Rajalakshmi Srinivasaraghavan wrote:
>This patch creates ifunc for sqrtf128() to make use of new xssqrtqp
>instruction for POWER9 when --enable-multi-arch and --with-cpu=power8
>options are used on power9 system. This is achieved by explicitly
>adding -mcpu=power9 flag for sqrtf128-power9.
>
>2018-05-17 Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>
>
> * sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile: New file to
> add w_sqrtf128-power9 and w_sqrtf128-ppc64le to libm-sysdep_routines.
> * sysdeps/powerpc/powerpc64/le/fpu/multiarch/w_sqrtf128-power9.c:
> New file.
> * sysdeps/powerpc/powerpc64/le/fpu/multiarch/w_sqrtf128-ppc64le.c:
> Likewise.
> * sysdeps/powerpc/powerpc64/le/fpu/multiarch/w_sqrtf128.c: Likewise.
This part looks good to me.
> * sysdeps/powerpc/powerpc64/le/power9/fpu/e_sqrtf128.c: Remove file
> as __ieee754_sqrtf128() for power9 is present already in
> sysdeps/powerpc/fpu/math_private.h.
This doesn't.
The function provided by e_sqrtf128.c (__sqrtf128_finite) is export by
GLIBC and accessible by users when they are building their code with
-ffast-math. When building glibc with --with-cpu=power8, removing this
file would be OK. However, when building glibc with --with-cpu=power9,
__sqrtf128_finite will use software emulation when it could use xssqrtqp.
Since math_private.h is not installed, it will not work for glibc users.
You patch is OK without this removal.
>+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/w_sqrtf128-power9.c
> (...)
>+#include <math-type-macros-float128.h>
> (...)
>+#undef __USE_WRAPPER_TEMPLATE
>+#define __USE_WRAPPER_TEMPLATE 1
Do you actually need this? What behaviour are you trying to override?
The float128 implementation always uses the wrapper templates.
>+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/w_sqrtf128-ppc64le.c
> (...)
>+#include <math-type-macros-float128.h>
> (...)
>+#undef __USE_WRAPPER_TEMPLATE
>+#define __USE_WRAPPER_TEMPLATE 1
Likewise.
>+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/w_sqrtf128.c
> (...)
>+libc_ifunc (__sqrtf128,
>+ (hwcap2 & PPC_FEATURE2_ARCH_3_00)
>+ ? __sqrtf128_power9
>+ : __sqrtf128_ppc64le);
>+declare_mgen_alias (__sqrt, sqrt)
OK.