On 07/20/2010 07:06 PM, Ivan Maidanski wrote: > Hello, Andrew! > >> On 06/29/2010 10:22 AM, Ivan Maidanski wrote: >>> Hi! >>> >>> I've fixed a number of bugs in StrictMath class (the RI here is fdlibm of >>> some version). >>> >>> ChangeLog entries: >>> >>> * java/lang/StrictMath.java: >>> (acos(double)): Bug fix for x <= -1/2 case. >>> (atan(double)): Fix documentation typo. >>> (pow(double,double)): Fix a comment; put y == 1/2 case handling after >>> x == -0 case (since pow(-0, 1/2) == 0 but sqrt(-0) == -0); return -0 >>> (or -INF) for pow(-0, 2*k) where k is non-zero integer; simplify >>> expression for "negative" variable. >>> (IEEEremainder(double,double)): Bug fix for x == -0 and x == -|y| >>> cases; bug fix for |y| >= 2**-1021 and |x| > |y|/2. >>> (remPiOver2(double[],double[],int,int)): Reset "recompute" at the >>> beginning of every do/while iteration. >>> (tan(double,double,boolean)): Negate the result if x <= -0.6744. >> >> Hi Ivan, >> >> Firstly please attach patches as type text; that way people can read >> them in their mailers. > > I don't understand. I attach patches with .diff extension. Shall I use .txt?
I think they had a mime-type of application/binary. >> Can you please send test cases that pass with this patch? > > Ok. I'll write them a bit later... > >> >> diff -ru CVS/classpath/java/lang/StrictMath.java >> updated/classpath/java/lang/StrictMath.java >> --- CVS/classpath/java/lang/StrictMath.java 2010-06-29 10:11:06.000000000 >> +0400 >> +++ updated/classpath/java/lang/StrictMath.java 2010-06-29 >> 12:51:50.000000000 +0400 >> @@ -1,5 +1,6 @@ >> /* java.lang.StrictMath -- common mathematical functions, strict Java >> - Copyright (C) 1998, 2001, 2002, 2003, 2006 Free Software Foundation, Inc. >> + Copyright (C) 1998, 2001, 2002, 2003, 2006, 2010 >> + Free Software Foundation, Inc. >> >> This file is part of GNU Classpath. >> >> @@ -456,9 +457,10 @@ >> double r = x - (PI_L / 2 - x * (p / q)); >> return negative ? PI / 2 + r : PI / 2 - r; >> } >> + double z = (1 - x) * 0.5; >> if (negative) // x<=-0.5. >> { >> - double z = (1 + x) * 0.5; >> + // z = (1+orig_x)*0.5 >> >> Please don't leave commented lines in code. If they're wrong, please >> take them out. > > This is not a commented code - it's a reference C code from fdlibm. It might > be better to use some words instead of a formular like "x is not modified in > fdlibm unlike in this implementation so we use 1-x (where x==-orig_x) instead > of 1+x". > > >> >> @@ -1554,10 +1553,18 @@ >> else if (yisint == 1) >> ax = -ax; >> } >> + else >> + { >> + // Check for x == -0 with odd y. >> + if (1 / x < 0 >> >> Why use "1 / x < 0" ? > > What's else to use? The alternative is Double.doubleToRawLongBits(x) > < 0 but (1/x)<0 is cheaper (recall that |x| is zero). In class T { boolean foo(double x) { return Double.doubleToRawLongBits(x) < 0; } boolean bar(double x) { return (1/x) < 0; } } gcj gives you T.foo(double)boolean: movsd %xmm0, -16(%rsp) movq -16(%rsp), %rax shrq $63, %rax ret for the first, and T.bar(double)boolean: movsd .LC0(%rip), %xmm1 divsd %xmm0, %xmm1 movapd %xmm1, %xmm0 xorpd %xmm1, %xmm1 ucomisd %xmm0, %xmm1 seta %al ret for the second. Any JIT might do something similar. IMO it's always better to say what you mean and let the optimizer worry about optimization. Andrew.