RFR (M): 8143353: Update for x86 sin and cos in the math lib

[Vladimir Kozlov]

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%28double%29

>
> 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?

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
>>>