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: [PATCH] Sparc exp(), expf() performance improvement


Apologies for the many earlier versions of this email.
My fingers forgot I was in a mailer not an editor and
every time I typed "^S" to save my temporary version,
the mailer did a send.

This is the intended version of this note.

- patrick


On 7/31/2017 4:06 PM, Joseph Myers wrote:
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.
Internally I need to support some Oracle Linux releases which use gcc 4.4.7.
Putting the test for -mcpu=niagara4 support allowed me to use one set of changes for
both the Oracle and sourceware versions.
I'll simplify my patch for sourceware, omitting the test.

Same applies to the use of
#ifndef DBL_MIN
as DBL_MIN was not defined in older glibc/gcc trees.
I'll remove the DBL_MIN def.



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).
Ok, removed. Will say "EXP function - Compute double precision exponential" for the comment.


+extern double exp (double);
No such declaration should be needed, since you're including <math.h>.
Ok, removed.

+extern double __ieee754_exp (double);
And this should come through math_private.h.
I'm not finding it in any math_private.h that I've checked.
Do you mean I should put it in sysdeps/sparc/fpu/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.
Fixed.
+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.

I agree that C99 hex float constants would be better than decimal, but I'm reluctant to change the formatting of any of the existing constants as it seems an easy opportunity to introduce a possible typo. Even something as simple as transposed digits in one table
entry could slip by even extended testing procedures.

On the other hand, I can readily change the zC table into the following:
static const half       0.5;
static const invln2_32 4.61662413084468283841e+01; /* 0x40471547, 0x652b82fe */
...
and add comments to the limit of my understanding of the algorithm.
I am porting this code, not the original developer.
I will make those changes.



+#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.).
will remove for sourceware version of code.
+#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.
will do.

Much the same comments apply to the float code.

I will make similar changes to float.


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