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.

Reply via email to