RFR (M): 8143353: Update for x86 sin and cos in the math lib
Deshpande, Vivek R
vivek.r.deshpande at intel.com
Tue Nov 24 18:22:15 UTC 2015
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%28dou
> 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