Re: [PATCH] Sparc exp(), expf() performance improvement

[Joseph Myers <joseph at codesourcery dot com>]

On Mon, 31 Jul 2017, Patrick McGehearty wrote:

>     * sysdeps/sparc/configure.ac: manage -mcpu=niagara4 test
>     * sysdeps/sparc/configure: Regenerate

As far as I can see, niagara4 support was added to GCC in r178554 
(2011-09-05), so would have been in GCC 4.7.  The minimum GCC version for 
building glibc is 4.9.  So this configure test, and any associated 
conditionals in Makefiles or elsewhere, should not be necessary.

> diff --git a/sysdeps/sparc/fpu/libm_endian.h b/sysdeps/sparc/fpu/libm_endian.h
> new file mode 100644
> index 0000000..a49abcd
> --- /dev/null
> +++ b/sysdeps/sparc/fpu/libm_endian.h
> @@ -0,0 +1,32 @@
> +/*
> +   Contributed by Oracle Inc.
> +   Copyright (C) 2017 Free Software Foundation, Inc.

All new files should start with a descriptive comment on their first line, 
before the copyright notice.  "Contributed by" is no longer used; 
significant contributions can be mentioned in the NEWS file (as well as in 
contrib.texi).

> +extern double exp (double);

No such declaration should be needed, since you're including <math.h>.

> +extern double __ieee754_exp (double);

And this should come through math_private.h.

> +/*
> + * For i = 0, ..., 66,
> + *   TBL2[2*i] is a double precision number near (i+1)*2^-6, and

We don't use leading '*' on each line of a comment.

> +static const double zC[] = {
> +  0.5,
> +  4.61662413084468283841e+01,	/* 0x40471547, 0x652b82fe */
> +  2.16608493865351192653e-02,	/* 0x3f962e42, 0xfee00000 */
> +  5.96317165397058656257e-12,	/* 0x3d9a39ef, 0x35793c76 */

Please put a comment on each file-scope variable or function explaining 
its semantics.

If you're commenting exact representations of floating-point numbers, I'd 
advise writing them as C99 hex float constants instead of decimal (and 
then omitting those comments) - generally I think hex float constants are 
a good idea for any constants that have been computed to have some 
property (polynomial coefficients, etc.) and can't be expected to be 
human-readable.

> +#ifndef DBL_MIN
> +#define DBL_MIN 2.2250738585072014E-308
> +#endif

Just include <float.h>.  No such #ifndef is then needed (and we use 
indentation inside #if, "# define" etc.).

> +#define	half		zC[0]
> +#define	invln2_32	zC[1]

It would be better to make each of these into a separate static const 
variable with its own comment rather than defining an array full of 
unrelated values.

Much the same comments apply to the float code.

-- 
Joseph S. Myers
joseph@codesourcery.com