RFR: 8189105: AARCH64: create intrinsic for sin and cos

Dmitrij Pochepko dmitrij.pochepko at bell-sw.com
Wed Jun 20 18:40:47 UTC 2018



On 20.06.2018 19:28, Andrew Haley wrote:
> On 06/20/2018 03:26 PM, Dmitrij Pochepko wrote:
>>
>> On 19.06.2018 20:46, Andrew Haley wrote:
>>> On 06/18/2018 07:53 PM, Dmitrij Pochepko wrote:
>>>> If you have any other ideas how to make the code easier to read and
>>>> maintain I’m happy to improve it.
>>> That's great!  Really, that's a vast improvement.  Excellent work.
>>>
>>> One or two relatively minor things:
>>>
>>> Why do you say the polynomial in kernel_sin/kernel_cos is a Taylor
>>> series?  There's no such comment in sharedRuntimeTrig.cpp, and I doubt
>>> it.  I would have thought it was a
>>> least-maximum polynomial
>>> approximation, not a Taylor series.
>> Taylor series for cos(x) = 1 - x^2/2! + x^4/4! - x^6/6! + ... = 1 -
>> 0.5*x^2 + 4.16666666666666019037e-02 * x^4 - 1.38888888888741095749e-03
>> * x^6 ...
>> so, if you take a look at kernel_cos function description you'll see
>> exactly same polynomial calculation with exactly same coefficients.
> I don't think so.  I think the program uses
>
>       4.16666666666666019037e-02, // c0x3FA555555555554C
>      -1.38888888888741095749e-03, // 0xBF56C16C16C15177
>       2.48015872894767294178e-05, // 0x3EFA01A019CB1590
>      -2.75573143513906633035e-07, // 0xBE927E4F809C52AD
>       2.08757232129817482790e-09, // 0x3E21EE9EBDB4B1C4
>      -1.13596475577881948265e-11  // 0xBDA8FAE9BE8838D4
>
> and unless I am mistaken the coefficients of the Taylor series for cos
> are approximately
>
>       4.16666666666666643537e-02,
>      -1.38888888888888894189e-03,
>       2.48015873015873015658e-05,
>      -2.75573192239858882758e-07,
>       2.08767569878681001866e-09,
>      -7.81894280887451282427e-10,
>    
Though your calculation of Taylor series coefficient is imprecise, you 
are right in your assessment this is not Taylor series. It's a 
polynomial coefficients derived by using Remez approximation algorithm, 
which typically use Taylor series coefficients as initial approximation.
I updated webrev by removing Taylor mentioning: 
http://cr.openjdk.java.net/~dpochepk/8189105/webrev.05/
>
>>> What's the point of keeping the doubles in tables in hex form?
>>> sharedRuntimeTrig.cpp doesn't do that, and it looks like obfuscation.
>>> Even if you must keep them as hex, at least move the comment which
>>> shows their true values next to their declaration, but I really can't
>>> see the point.
>> It was easier for me while debugging to use hex form. I changed
>> representation to double with hex form added in comment where applicable.
> Excellent.
>


More information about the hotspot-compiler-dev mailing list