Hi Volker,
On 9/16/2015 2:02 AM, Volker Simonis wrote:
Hi Joe,
nice clean-up! From a maintenance point of view the the old fdlibm has
been a burden and a constant source of warnings with newer compilers.
What about the final arrays in Pow.compute (BP, DP_H, DP_L). In the
Interpreter (and even in C2 without EscapeAnalysis) this will lead to
allocations every time the method is called. Making them static fields
of 'Pow' will help here. On the other hand, if the arrays are static
fields of 'Pow', C2 will not be able to directly use the constants but
will have to load them from the array. So altogether, your current
implementation is optimal with HotSpot (with EscapeAnalysis turned on
by default) but might not be the best for other VMs without
EscapaAnalysis.
Have you done any benchmarking which compares the new Java against the
old C implementation?
I don't have exact stats to share, but some quick benchmarks indicate it
is noticeably faster, more than 10%. The less computation pow does in a
given case, the faster the Java version should be since the Java -> C ->
Java cost should roughly be fixed.
Once the code is fully in Java, I would expect further tweaks to refine
the code. For example, the original FDLIM code used the idiom of pointer
aliasing a two-element int array with a double. (This idiom was also the
source of many of those compiler warnings ;-) We can't directly use that
idiom in Java, but the early versions of the port were a close
transliteration of the C sources with equivalent Java method calls. That
still left the code as operating on 32-bit quantities or doing
comparisons of the double value by looking at its 32 high-order bits.
Many of those 32-bit operations were removed as the port progressed to
use clearer (if not also faster) operations such as comparing 64-bit
double values directly as 64-bit double values.
For example, to my eye I'd much rather read
185 // Special value of x
186 if (x_abs == 0.0 ||
187 x_abs == INFINITY ||
188 x_abs == 1.0) {
189 z = x_abs; // x is +/-0, +/-inf, +/-1
190 if (y < 0.0)
191 z = 1.0/z; // z = (1/|x|)
where x_abs is a double holding the absolute value of x, than the
original semantically equivalent original
178 /* special value of x */
179 if(lx==0) {
180 if(ix==0x7ff00000||ix==0||ix==0x3ff00000){
181 z = ax; /*x is +-0,+-inf,+-1*/
182 if(hy<0) z = one/z; /* z = (1/|x|) */
where lx is the lower-order 32 bits of x and ix is the high-order 32
bits of x with the sign bit zeroed out.
The version sent out for review still has some remaining 32-bit centered
operations and at least some of those could be eliminated.
Thanks,
-Joe
