This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [v2 2/2] PPC64: Add libmvec SIMD double-precision natural exponent function.


Hi Shawn,

This patch is a lot better.

Shawn Landden <shawn@git.icu> writes:

> Passes all tests.
>
> Unlike other libmvec functions, this sets the onderflow and overflow bits.

s/onderflow/underflow/

> The caller can check these flags, and possibly re-run the calculations with
> scalar expf to figure out what is causing the overflow or underflow.
>
> The special-case path is not vectorized, and performs much woorse than

s/woorse/worse/

> 2019-05-11  Shawn Landden  <shawn@git.icu>
>
> 	[BZ #24209]
> 	* NEWS: Noted the addition of PPC64 vector exp function.
> 	* sysdeps/powerpc/bits/math-vector.h: Added entry for vector exp.
> 	* sysdeps/powerpc/powerpc64/fpu/Versions: Added vector exp entry.
> 	* sysdeps/powerpc/powerpc64/fpu/multiarch/Makefile:
> 	(libmvec-sysdep_routines, CFLAGS-vec_d_exp2_vsx.c):
> 	(CFLAGS-vec_d_exp_data.c): Added build of VSX SIMD expf
> 	function and tests. Added vec_math_err.c to build.
> 	* sysdeps/powerpc/powerpc64/fpu/multiarch/math_config.h: Renamed to ...
>         * sysdeps/powerpc/powerpc64/fpu/multiarch/math_config_dbl.h: ... and
>         modified for exp.
>         * sysdeps/powerpc/powerpc64/fpu/multiarch/test-double-vlen2-wrappers.c:
>         Added entry for vector exp.
>         * sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp2_vsx.c: New file.
>         * sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp_data.c: Likewise.
>         * sysdeps/powerpc/powerpc64/fpu/multiarch/vec_math_err.c: Likewise.
>         * sysdeps/unix/sysv/linux/powerpc/powerpc64/libmvec.abilist: SIMD expf
>         added.
> 	* sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_log_data.c: Change to
>         reflect rename of math_config.h
>         * sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_log2_vsx.c: Likewise

There are still 11 lines indented with spaces instead of tab here.
You can identify easily in this quote.

> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/Makefile b/sysdeps/powerpc/powerpc64/fpu/multiarch/Makefile
> index ad3c29b1ab..aceaedee0a 100644
> --- a/sysdeps/powerpc/powerpc64/fpu/multiarch/Makefile
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/Makefile
> @@ -47,11 +47,12 @@ ifeq ($(subdir),mathvec)
>  libmvec-sysdep_routines += vec_d_cos2_vsx vec_s_cosf4_vsx \
>  			   vec_d_sin2_vsx vec_s_sinf4_vsx \
>  			   vec_d_log2_vsx vec_d_log_data \
>  			   vec_s_logf4_vsx vec_s_logf_data \
>  			   vec_s_expf4_vsx vec_s_exp2f_data \
> -			   vec_math_errf \
> +			   vec_math_errf vec_math_err \
> +			   vec_d_exp2_vsx vec_d_exp_data \
>  			   vec_d_sincos2_vsx vec_s_sincosf4_vsx
>  CFLAGS-vec_d_cos2_vsx.c += -mabi=altivec -maltivec -mvsx -mpower8-vector
>  CFLAGS-vec_d_log2_vsx.c += -mabi=altivec -maltivec -mvsx -mpower8-vector
>  CFLAGS-vec_d_log_data.c += -mabi=altivec -maltivec -mvsx -mpower8-vector
>  CFLAGS-vec_s_cosf4_vsx.c += -mabi=altivec -maltivec -mvsx
> @@ -61,18 +62,20 @@ CFLAGS-vec_d_sin2_vsx.c += -mabi=altivec -maltivec -mvsx -mpower8-vector
>  CFLAGS-vec_s_sinf4_vsx.c += -mabi=altivec -maltivec -mvsx
>  CFLAGS-vec_d_sincos2_vsx.c += -mabi=altivec -maltivec -mvsx -mpower8-vector
>  CFLAGS-vec_s_sincosf4_vsx.c += -mabi=altivec -maltivec -mvsx
>  CFLAGS-vec_s_expf4_vsx.c += -mabi=altivec -maltivec -mvsx
>  CFLAGS-vec_s_exp2f_data.c += -mabi=altivec -maltivec -mvsx
> +CFLAGS-vec_d_exp2_vsx.c += -mabi=altivec -maltivec -mvsx -mpower8-vector
> +CFLAGS-vec_d_exp_data.c += -mabi=altivec -maltivec -mvsx

vec_math_err.c includes altivec.h indirectly, requiring the same CFLAGS.

> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp2_vsx.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp2_vsx.c
> new file mode 100644
> index 0000000000..5d59cc429d
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp2_vsx.c
> @@ -0,0 +1,204 @@
> +/* Double-precision vector exp function.
> +   Copyright 2019 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.
> +   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/>.  */

We need to reuse the copyright template that is available in the file that
you used as a base for this one.  You just need to update the year.

That means to keep "(C)".

> +/* Based on sysdeps/ieee754/dbl-64/e_exp.c which came from
> +   Szabolcs Nagy at ARM Ltd.  */

Thanks for adding this.

> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp_data.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp_data.c
> new file mode 100644
> index 0000000000..8038a7b592
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_d_exp_data.c
> @@ -0,0 +1,211 @@
> +/* Shared data between exp, exp2 and pow.

Should this be just exp?

> +
> +   Copyright 2019 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.
> +   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/>.  */

Same comment here.

> +/* Based on ./sysdeps/ieee754/dbl-64/e_exp_data.c which came from
> +   Szabolcs Nagy at ARM Ltd.  */

Thanks again.

> +// Used for rounding when !TOINT_INTRINSICS
> +#if EXP_USE_TOINT_NARROW

This usage is still lost here and causes a build failure.

> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_math_err.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_math_err.c
> new file mode 100644
> index 0000000000..2f88b81ab2
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/vec_math_err.c
> @@ -0,0 +1,41 @@
> ...
> +attribute_hidden double
> +__math_oflow (uint32_t sign)
> +{
> +  return xflow (sign, 0x1p769);
> +}
> +

Extra line at EOF.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libmvec.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libmvec.abilist
> index 63770c8da3..26175bb1a0 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libmvec.abilist
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libmvec.abilist
> @@ -4,6 +4,7 @@ GLIBC_2.30 _ZGVbN2v_sin F
>  GLIBC_2.30 _ZGVbN2vvv_sincos F
>  GLIBC_2.30 _ZGVbN4v_cosf F
>  GLIBC_2.30 _ZGVbN4v_logf F
>  GLIBC_2.30 _ZGVbN4v_sinf F
>  GLIBC_2.30 _ZGVbN4vvv_sincosf F
> +GLIBC_2.30 _ZGVbN2v_exp F

This is still causing an issue with mathvec/check-abi-libmvec

-- 
Tulio Magno


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]