RFR (M): 8143353: Update for x86 sin and cos in the math lib
Deshpande, Vivek R
vivek.r.deshpande at intel.com
Wed Jan 20 00:44:25 UTC 2016
Hi
According LIBM experts at Intel for the test cases,
the data sets used in regression tests for the Intel the math library (libm).
They were collected over a long period of testing various libm implementations.
The data sets contain function specific data (special and corner cases such as +/-0,
maximum/minimum normalized numbers, +/-infinity, QNaN/SNaN, maximum/minimum denormal numbers,
arguments that would produce close to overflow/underflow results, known hard-to-round cases, etc),
implementation specific data (arguments close to table look-up values for different polynomial approximations,
worst cases for range reduction algorithms) and other data with interesting bit patterns.
Regards,
Vivek
-----Original Message-----
From: joe darcy [mailto:joe.darcy at oracle.com]
Sent: Friday, January 15, 2016 6:29 PM
To: Vladimir Kozlov; Deshpande, Vivek R
Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in the math lib
Ah okay; I overlooked the separate push of the tests.
Thanks,
-Joe
On 1/15/2016 5:58 PM, Vladimir Kozlov wrote:
> Note, the test was pushed together with VM changes into hs-comp repo:
>
> http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/ddd59a780769
>
> New sin/cos code is tested in all running modes since it is used by
> Interpreter and JITed code (C1 and C2).
>
> I will let Vivek answer questions about the test.
>
> Regards,
> Vladimir
>
> On 1/15/16 5:33 PM, Joseph D. Darcy wrote:
>> Hello,
>>
>> Catching up on email, how were these test cases generated or chosen?
>> In other words, in what sense are they corners?
>>
>> The data would be easier to read if the numbers were aligned by
>> column (they don't appear that way in the webrev at least).
>>
>> What is the code coverage of the new intrinsics with this set of tests?
>>
>> Theses tests should not be separated from the implementation for
>> long; in other words, since the new implementation has already been
>> pushed to a HotSpot forest, test coverage for that new implementation
>> should not lag behind.
>>
>> Thanks,
>>
>> -Joe
>>
>> On 12/22/2015 5:41 PM, Deshpande, Vivek R wrote:
>>> HI All
>>>
>>> I have uploaded the patch for sin and cos tests with input and
>>> allowed outputs at this location for your review.
>>> http://cr.openjdk.java.net/~vdeshpande/libm_sincos/8143353/jdk/webre
>>> v.00/
>>>
>>> Bug ID: https://bugs.openjdk.java.net/browse/JDK-8143353
>>> Thank you.
>>>
>>> Regards,
>>> Vivek
>>>
>>> -----Original Message-----
>>> From: Joseph D. Darcy [mailto:joe.darcy at oracle.com]
>>> Sent: Friday, December 04, 2015 4:50 PM
>>> To: Deshpande, Vivek R; Vladimir Kozlov
>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>> Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in the
>>> math lib
>>>
>>> Hi Vivek,
>>>
>>> On 12/3/2015 2:01 PM, Deshpande, Vivek R wrote:
>>>> Hi
>>>>
>>>> Sure I will add the tests. Shall I use StrictMath result as a
>>>> reference for exact result.
>>>> Let me know your thoughts.
>>> As a rough test of another sin/cos implementation, StrictMath.{sin,
>>> cos} can be used a reference with the following caveat: there isn't
>>> an indication of which why the error is in a StrictMath result. Let
>>> me given an example, if
>>>
>>> StrictMath.sin(x) => y
>>>
>>> then one of the following should be true
>>>
>>> Math.sin(x) => y
>>> Math.sin(x) => Math.nextUp(y)
>>> Math.sin(x) => Math.nextDown(y)
>>>
>>> That is, Math.sin(x) should either be the same as StrictMath.sin(x)
>>> OR equal to one of the floating-point numbers adjacent to that
>>> result. Of these three options, only two area allowed by the
>>> accuracy requirements of the StrictMath.sin specification. However,
>>> since StrictMath.sin doesn't give an indication of which way its
>>> error went (if it rounded up or down), there is no indication
>>> without additional work which of
>>> nextUp(y) and nextDown(y) is allowable (assuming StrictMath.sin
>>> isn't buggy).
>>>
>>> HTH,
>>>
>>> -Joe
>>>
>>>
>>>> Regards,
>>>> Vivek
>>>>
>>>> -----Original Message-----
>>>> From: joe darcy [mailto:joe.darcy at oracle.com]
>>>> Sent: Thursday, December 03, 2015 1:29 PM
>>>> To: Vladimir Kozlov; Deshpande, Vivek R
>>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>>> Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in the
>>>> math lib
>>>>
>>>> Hello,
>>>>
>>>> On 12/3/2015 1:25 PM, Vladimir Kozlov wrote:
>>>>> Vivek,
>>>>>
>>>>> I think Joe is asking you to write these tests as hotspot
>>>>> regression test in hotspot/test/compiler.
>>>> Exactly; if not generally applicable sin/cos tests that could be
>>>> hosted in the jdk repo (alongside the regression and unit tests for
>>>> java.lang.Math), then test of intrinsics in the HotSpot repo
>>>> alongside other tests targeting intrinsics.
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>>> Vladimir
>>>>>
>>>>> On 12/3/15 1:22 PM, Deshpande, Vivek R wrote:
>>>>>> Hi Joe
>>>>>>
>>>>>> It would be great if you would please share the additional tests
>>>>>> with us.
>>>>>>
>>>>>> Regards,
>>>>>> Vivek
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: joe darcy [mailto:joe.darcy at oracle.com]
>>>>>> Sent: Thursday, December 03, 2015 1:17 PM
>>>>>> To: Vladimir Kozlov; Deshpande, Vivek R
>>>>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>>>>> Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in the
>>>>>> math lib
>>>>>>
>>>>>> I think it is unwise for this large of an implementation change
>>>>>> to be pushed with no tests targeting the specifics of the new
>>>>>> implementation.
>>>>>>
>>>>>> The worst-case tests in the jdk repo are the mathematical worst
>>>>>> cases for floating-point approximations, in other words the cases
>>>>>> were the exact mathematical answer is closes to half-way between
>>>>>> two representation floating-point numbers. Passing such tests is
>>>>>> necessary but not sufficient condition for a new implementation.
>>>>>>
>>>>>> Chers,
>>>>>>
>>>>>> -Joe
>>>>>>
>>>>>> On 12/3/2015 1:05 PM, Vladimir Kozlov wrote:
>>>>>>> Okay, looks reasonable to me.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 12/3/15 11:06 AM, Deshpande, Vivek R wrote:
>>>>>>>> Hi Vladimir
>>>>>>>>
>>>>>>>> This is the link for the updated webrev with latest hotspot
>>>>>>>> source as base for your review.
>>>>>>>> http://cr.openjdk.java.net/~mcberg/8143353/webrev.03/
>>>>>>>> Thank you.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Vivek
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Deshpande, Vivek R
>>>>>>>> Sent: Wednesday, December 02, 2015 10:33 PM
>>>>>>>> To: 'Vladimir Kozlov'; joe darcy
>>>>>>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>>>>>>> Subject: RE: RFR (M): 8143353: Update for x86 sin and cos in
>>>>>>>> the math lib
>>>>>>>>
>>>>>>>> Hi Vladimir
>>>>>>>>
>>>>>>>> This is the link for the updated webrev for your review.
>>>>>>>> http://cr.openjdk.java.net/~mcberg/8143353/webrev.02/
>>>>>>>> Thank you.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Vivek
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>>> Sent: Tuesday, December 01, 2015 6:06 PM
>>>>>>>> To: Deshpande, Vivek R; joe darcy
>>>>>>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>>>>>>> Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in
>>>>>>>> the math lib
>>>>>>>>
>>>>>>>> Please send link to new webrev on cr server.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> On 11/25/15 5:16 PM, Deshpande, Vivek R wrote:
>>>>>>>>> Hi Vladimir
>>>>>>>>>
>>>>>>>>> Please find the webrev with your suggested updates attached
>>>>>>>>> with the mail.
>>>>>>>>> We will update it in the jbs entry soon.
>>>>>>>>> Please let me know if it needs further changes.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Vivek
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Deshpande, Vivek R
>>>>>>>>> Sent: Tuesday, November 24, 2015 10:22 AM
>>>>>>>>> To: 'joe darcy'; Vladimir Kozlov
>>>>>>>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>>>>>>>> Subject: RE: RFR (M): 8143353: Update for x86 sin and cos in
>>>>>>>>> the math lib
>>>>>>>>>
>>>>>>>>> HI Vladimir, Joe
>>>>>>>>>
>>>>>>>>> I have done the jtreg tests in hotspot and tests from jdk you
>>>>>>>>> have mentioned. It passed those tests.
>>>>>>>>> The ~4x gain is with XX:+UnlockDiagnosticVMOptions
>>>>>>>>> -XX:DisableIntrinsic=_dsin/_dcos over without that option.
>>>>>>>>> The performance gain is 3.2x over base jdk, that is over
>>>>>>>>> current fsin/fcos intrinsic. This gain is more realistic.
>>>>>>>>>
>>>>>>>>> Could I get those tests around the boundary values. Would
>>>>>>>>> WorstCaseTests.java jtreg test in jdk test those ?
>>>>>>>>> If yes, then it has passed those boundary cases.
>>>>>>>>>
>>>>>>>>> I would work on adding either diagnostic flag or just one flag
>>>>>>>>> for libm and send out the webrev soon.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Vivek
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: joe darcy [mailto:joe.darcy at oracle.com]
>>>>>>>>> Sent: Monday, November 23, 2015 6:28 PM
>>>>>>>>> To: Vladimir Kozlov; Deshpande, Vivek R
>>>>>>>>> Cc: Viswanathan, Sandhya; Berg, Michael C; hotspot compiler
>>>>>>>>> Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in
>>>>>>>>> the math lib
>>>>>>>>>
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Just getting added to the thread..
>>>>>>>>>
>>>>>>>>> On 11/23/2015 5:13 PM, Vladimir Kozlov wrote:
>>>>>>>>>> Thank you, for explanation, Vivek.
>>>>>>>>>>
>>>>>>>>>> Please, run jdk/test/java/lang/Math/ jtreg tests in addition
>>>>>>>>>> to Hotspot tests.
>>>>>>>>>>
>>>>>>>>>> On 11/23/15 12:24 PM, Deshpande, Vivek R wrote:
>>>>>>>>>>> Hi Vladimir
>>>>>>>>>>>
>>>>>>>>>>> The result we obtain with LIBM are within +/- 1ulp from
>>>>>>>>>>> StrictMath result and not exact result. So I added the flag
>>>>>>>>>>> to switch between FDLIBM and LIBM.
>>>>>>>>>>>
>>>>>>>>>>> Quick explanation:
>>>>>>>>>>> This is what we observed with comparison to HPA Library
>>>>>>>>>>> (http://www.nongnu.org/hpalib/) explained with an example.
>>>>>>>>>>> LIBM Observed Math result=0.19457293629570213
>>>>>>>>>>> (4596178249117717083L) (StrictMath - 1ulp) Required result
>>>>>>>>>>> should be = 0.19457293629570216
>>>>>>>>>>> (4596178249117717084L) (StrictMath result) or
>>>>>>>>>>> 0.1945729362957022
>>>>>>>>>>> (4596178249117717085L) (StrictMath + 1ulp.) This means HPA
>>>>>>>>>>> library result is between the above two values and Exact
>>>>>>>>>>> result would be pretty close to it.
>>>>>>>>>>> So here StrictMath result is less than quad-precision
>>>>>>>>>>> result, Math result should be StrictMath or StrictMath +
>>>>>>>>>>> 1ulp and not StrictMath
>>>>>>>>>>> - 1ulp, according to our test.
>>>>>>>>>> Note, java.lang.Math allows to have 1ulp off (in both
>>>>>>>>>> direction, I
>>>>>>>>>> think) and it should be consistent for Interpreter and code
>>>>>>>>>> generated by JIT compilers:
>>>>>>>>>>
>>>>>>>>>> http://docs.oracle.com/javase/7/docs/api/java/lang/Math.html#
>>>>>>>>>> sin
>>>>>>>>>> %
>>>>>>>>>> 28
>>>>>>>>>> do
>>>>>>>>>> u
>>>>>>>>>> ble%29
>>>>>>>>>>
>>>>>>>>> That interpretation of the spec is not quite right. For the
>>>>>>>>> Math methods with a 1/2 ulp error bound, the floating-point
>>>>>>>>> result closest to the exact result must be returned. For the
>>>>>>>>> methods with a
>>>>>>>>> 1 ulp error bound, either of the floating-point result
>>>>>>>>> bracketing the true result can be returned, subject to the
>>>>>>>>> monotonicity constraints of the specification of the particular method.
>>>>>>>>>
>>>>>>>>>>> I have done the experiments with
>>>>>>>>>>> XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_dsin and
>>>>>>>>>>> XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_dcos.
>>>>>>>>>>> With this option, the interpreter would go through LIBM and C1 and c2 through FDLIBM.
>>>>>>>>>>> If we want to disable LIBM completely, we need the flags
>>>>>>>>>>> -XX:+UseLibmSinIntrinsic and -XX:+UseLibmCosIntrinsic.
>>>>>>>>>> I was thinking about using existing
>>>>>>>>>> DirectiveSet::is_intrinsic_disabled() and
>>>>>>>>>> vmIntrinsics::is_disabled_by_flags(). You need to add
>>>>>>>>>> additional versions of functions which accept intrinsic ID
>>>>>>>>>> instead of methodHandle.
>>>>>>>>>>
>>>>>>>>>> If you still want to use flags make them diagnostic.
>>>>>>>>>> Or have one flag for all LIBM intrinsics -XX:+UseLibmIntrinsic.
>>>>>>>>>>
>>>>>>>>>>> Also the performance gain ~4x is with
>>>>>>>>>>> XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_dsin/_dcos.
>>>>>>>>>> You confused me here. So you get 4x when only Interpreter use
>>>>>>>>>> LIBM code and compilers use FDLIB?
>>>>>>>>> Just to be clear, are you comparing the new code to FDLIBM
>>>>>>>>> (StrictMath) or to the existing fsin/fcos instrinsics (Math)?
>>>>>>>>>
>>>>>>>>> I'm part way through porting the FDLIBM code to Java
>>>>>>>>> (JDK-8134780:
>>>>>>>>> Port fdlibm to Java), which is providing a significant speed
>>>>>>>>> boost to the StrictMath methods that have been ported.
>>>>>>>>>
>>>>>>>>> I find the current patch *insufficient* as-is in terms of its
>>>>>>>>> testing.
>>>>>>>>> For example, part of patch says
>>>>>>>>>
>>>>>>>>> # For sin
>>>>>>>>>
>>>>>>>>> +// This means that the main path is actually only taken for
>>>>>>>>> +// 2^-252 <= |X| < 90112.
>>>>>>>>>
>>>>>>>>> # For cos
>>>>>>>>>
>>>>>>>>> +// This means that the main path is actually only taken for
>>>>>>>>> +// 2^-252 <= |X| < 90112.
>>>>>>>>>
>>>>>>>>> If nothing else, there are no tests at around those boundary
>>>>>>>>> values, which is unacceptable. There should also be some tests
>>>>>>>>> of values of interest to the algorithm in question.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>> -Joe
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Vladimir
>>>>>>>>>>
>>>>>>>>>>> Let me know your thoughts on this. I would answer more
>>>>>>>>>>> questions and give more data if needed.
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Vivek
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>>>>>> Sent: Monday, November 23, 2015 10:37 AM
>>>>>>>>>>> To: Deshpande, Vivek R;
>>>>>>>>>>> hotspot-compiler-dev at openjdk.java.net
>>>>>>>>>>> Cc: Viswanathan, Sandhya
>>>>>>>>>>> Subject: Re: RFR (M): 8143353: Update for x86 sin and cos in
>>>>>>>>>>> the math lib
>>>>>>>>>>>
>>>>>>>>>>> On 11/20/15 12:22 PM, Vladimir Kozlov wrote:
>>>>>>>>>>>> What is the reason you decided to add new flags? exp() and
>>>>>>>>>>>> log() changes did not have flags.
>>>>>>>>>>>>
>>>>>>>>>>>> It would be interesting to see what happens if you disable
>>>>>>>>>>>> intrinsics using existing flag, for example:
>>>>>>>>>>>>
>>>>>>>>>>>> -XX:+UnlockDiagnosticVMOptions
>>>>>>>>>>>> -XX:DisableIntrinsic=_dexp
>>>>>>>>>>> Hi Vivek,
>>>>>>>>>>>
>>>>>>>>>>> I want to point that you can do this experiment later. We
>>>>>>>>>>> can file bugs and fixed them after FC.
>>>>>>>>>>>
>>>>>>>>>>> For now, please, answer my question about flags only. This
>>>>>>>>>>> is the only thing holding it from push.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Vladimir
>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Vladimir
>>>>>>>>>>>>
>>>>>>>>>>>> On 11/20/15 12:03 PM, Deshpande, Vivek R wrote:
>>>>>>>>>>>>> Hi all
>>>>>>>>>>>>>
>>>>>>>>>>>>> I would like to contribute a patch which optimizes
>>>>>>>>>>>>> Math.sin() and
>>>>>>>>>>>>> Math.cos() for 64 and 32 bit X86 architecture using Intel
>>>>>>>>>>>>> LIBM
>>>>>>>>>>>>> implementation.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The improvement gives ~4.25x gain over base for both sin
>>>>>>>>>>>>> and cos.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The option to use the optimizations are
>>>>>>>>>>>>> -XX:+UseLibmSinIntrinsic and -XX:+UseLibmCosIntrinsic.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Could you please review and sponsor this patch.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Bug-id:
>>>>>>>>>>>>>
>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8143353
>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~mcberg/8143353/webrev.01/
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks and regards,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Vivek
>>>>>>>>>>>>>
>>
More information about the hotspot-compiler-dev
mailing list