RFR (M): 8150767: Update for x86 SHA Extensions enabling
Deshpande, Vivek R
vivek.r.deshpande at intel.com
Thu Mar 3 22:43:50 UTC 2016
Hi Vladimir, Christian
I have updated the code with #ifdef.
Please find the updated webrev here:
http://cr.openjdk.java.net/~vdeshpande/SHANI/8150767/webrev.03/
Regards,
Vivek
-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
Sent: Wednesday, March 02, 2016 2:14 PM
To: Christian Thalinger; Deshpande, Vivek R
Cc: hotspot compiler; Rukmannagari, Shravya
Subject: Re: RFR (M): 8150767: Update for x86 SHA Extensions enabling
On 3/2/16 2:04 PM, Christian Thalinger wrote:
>
>> On Mar 2, 2016, at 11:49 AM, Deshpande, Vivek R
>> <vivek.r.deshpande at intel.com <mailto:vivek.r.deshpande at intel.com>> wrote:
>>
>> Hi Christian
>>
>> We could combine the declaration of the functions such as
>> fast_sha256() for 32 bit and 64 bit in macroAssembler_x86.hpp using COMMA.
>> We used COMMA to separate more arguments used in 64 bit using
>> LP64_ONLY macro.
>
> Ugh. This is ugly:
>
> voidfast_pow(XMMRegisterxmm0, XMMRegisterxmm1, XMMRegisterxmm2,
> XMMRegisterxmm3, XMMRegisterxmm4, XMMRegisterxmm5, XMMRegisterxmm6,
> XMMRegisterxmm7, Registerrax, Registerrcx, Register rdx NOT_LP64(COMMA
> Register tmp) LP64_ONLY(COMMA Register tmp1)
> LP64_ONLY(COMMA Register tmp2) LP64_ONLY(COMMA
> Register
> tmp3) LP64_ONLY(COMMA Register tmp4));
>
> Can’t we use #ifdef instead?
+1 for #ifdef
It is not first time we scope additional parameters with #ifdef.
Vladimir
>
>>
>> Regards,
>> Vivek
>>
>> -----Original Message-----
>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>> Sent: Wednesday, March 02, 2016 9:30 AM
>> To: Deshpande, Vivek R
>> Cc: Vladimir Kozlov; hotspot compiler; Rukmannagari, Shravya
>> Subject: Re: RFR (M): 8150767: Update for x86 SHA Extensions enabling
>>
>> #define COMMA ,
>>
>> Why do we have a define like this? It came in with 8139575.
>>
>>> On Mar 1, 2016, at 3:24 PM, Deshpande, Vivek R
>>> <vivek.r.deshpande at intel.com <mailto:vivek.r.deshpande at intel.com>> wrote:
>>>
>>> Hi Vladimir, Christian
>>>
>>> I have updated the code according your suggestion of file name change.
>>> The updated webrev is at this location:
>>> http://cr.openjdk.java.net/~vdeshpande/SHANI/8150767/webrev.02/
>>> Please let me know if I have to do anything more.
>>> Also is there any change required to Makefile so that
>>> configurations.xml has name of the added file ?
>>>
>>> Regards,
>>> Vivek
>>>
>>> -----Original Message-----
>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>> Sent: Monday, February 29, 2016 5:59 PM
>>> To: Vladimir Kozlov
>>> Cc: Deshpande, Vivek R; hotspot compiler; Rukmannagari, Shravya
>>> Subject: Re: RFR (M): 8150767: Update for x86 SHA Extensions
>>> enabling
>>>
>>>
>>>> On Feb 29, 2016, at 3:15 PM, Vladimir Kozlov
>>>> <vladimir.kozlov at oracle.com> wrote:
>>>>
>>>> I am against to have "intel" in a file name. We have
>>>> macroAssembler_libm_x86_*.cpp files for math intrinsics which does
>>>> not have intel in name. So I prefer to not have it. I would suggest
>>>> macroAssembler_sha_x86.cpp.
>>>
>>> I know we already have macroAssembler_libm_x86_*.cpp but
>>> macroAssembler_x86_<feature>.cpp would be better.
>>>
>>>> You can manipulate when to use it in vm_version_x86.cpp.
>>>>
>>>> Intel Copyright in the file's header is fine.
>>>>
>>>> Code changes are fine now (webrev.01).
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 2/29/16 4:42 PM, Christian Thalinger wrote:
>>>>>
>>>>>> On Feb 29, 2016, at 2:00 PM, Deshpande, Vivek R
>>>>>> <vivek.r.deshpande at intel.com> wrote:
>>>>>>
>>>>>> Hi Christian
>>>>>>
>>>>>> We used the SHA Extension
>>>>>> implementations(https://software.intel.com/en-us/articles/intel-s
>>>>>> ha-extensions-implementations) for the JVM implementation of SHA1
>>>>>> and SHA256.
>>>>>
>>>>> Will that extension only be available on Intel chips?
>>>>>
>>>>>> It needed to have Intel copyright, so created a separate file.
>>>>>
>>>>> That is reasonable.
>>>>>
>>>>>> The white paper for the implementation is
>>>>>> https://software.intel.com/sites/default/files/article/402097/intel-sha-extensions-white-paper.pdf.
>>>>>>
>>>>>> Regards,
>>>>>> Vivek
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>> Sent: Monday, February 29, 2016 1:58 PM
>>>>>> To: Deshpande, Vivek R
>>>>>> Cc: Vladimir Kozlov; hotspot compiler; Rukmannagari, Shravya
>>>>>> Subject: Re: RFR (M): 8150767: Update for x86 SHA Extensions
>>>>>> enabling
>>>>>>
>>>>>> Why is the new file called macroAssembler_intel_x86.cpp?
>>>>>>
>>>>>>> On Feb 29, 2016, at 11:29 AM, Deshpande, Vivek R
>>>>>>> <vivek.r.deshpande at intel.com> wrote:
>>>>>>>
>>>>>>> HI Vladimir
>>>>>>>
>>>>>>> Thank you for your review.
>>>>>>> I have updated the patch with the changes you have suggested.
>>>>>>> The new webrev is at this location:
>>>>>>> http://cr.openjdk.java.net/~vdeshpande/SHANI/8150767/webrev.01/
>>>>>>>
>>>>>>> Regards
>>>>>>> Vivek
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>> Sent: Friday, February 26, 2016 6:50 PM
>>>>>>> To: Deshpande, Vivek R; hotspot compiler
>>>>>>> Cc: Viswanathan, Sandhya; Rukmannagari, Shravya
>>>>>>> Subject: Re: RFR (M): 8150767: Update for x86 SHA Extensions
>>>>>>> enabling
>>>>>>>
>>>>>>> Very nice, Vivek!!!
>>>>>>>
>>>>>>> Did you run tests with both 32- and 64-bit VMs?
>>>>>>>
>>>>>>> Small notes:
>>>>>>>
>>>>>>> In vm_version_x86.hpp spacing are not aligned in next line:
>>>>>>>
>>>>>>> static bool supports_avxonly() { return ((supports_avx2() ||
>>>>>>> supports_avx()) && !supports_evex()); }
>>>>>>> + static bool supports_sha() { return (_features & CPU_SHA)
>>>>>>> != 0; }
>>>>>>>
>>>>>>> Flags setting code in vm_version_x86.cpp should be like this
>>>>>>> (you can check supports_sha() only once, don't split '} else {'
>>>>>>> line, set UseSHA false if all intrinsics flags are false (I
>>>>>>> included UseSHA512Intrinsics for future) ):
>>>>>>>
>>>>>>> if (supports_sha()) {
>>>>>>> if (FLAG_IS_DEFAULT(UseSHA)) {
>>>>>>> UseSHA = true;
>>>>>>> }
>>>>>>> } else if (UseSHA) {
>>>>>>> warning("SHA instructions are not available on this CPU");
>>>>>>> FLAG_SET_DEFAULT(UseSHA, false); }
>>>>>>>
>>>>>>> if (UseSHA) {
>>>>>>> if (FLAG_IS_DEFAULT(UseSHA1Intrinsics)) {
>>>>>>> FLAG_SET_DEFAULT(UseSHA1Intrinsics, true); } } else if
>>>>>>> (UseSHA1Intrinsics) { warning("Intrinsics for SHA-1 crypto hash
>>>>>>> functions not available on this CPU.");
>>>>>>> FLAG_SET_DEFAULT(UseSHA1Intrinsics, false); }
>>>>>>>
>>>>>>> if (UseSHA) {
>>>>>>> if (FLAG_IS_DEFAULT(UseSHA256Intrinsics)) {
>>>>>>> FLAG_SET_DEFAULT(UseSHA256Intrinsics, true); } } else if
>>>>>>> (UseSHA256Intrinsics) { warning("Intrinsics for SHA-224 and
>>>>>>> SHA-256 crypto hash functions not available on this CPU.");
>>>>>>> FLAG_SET_DEFAULT(UseSHA256Intrinsics, false); }
>>>>>>>
>>>>>>> if (UseSHA512Intrinsics) {
>>>>>>> warning("Intrinsics for SHA-384 and SHA-512 crypto hash
>>>>>>> functions not available on this CPU.");
>>>>>>> FLAG_SET_DEFAULT(UseSHA512Intrinsics, false); }
>>>>>>>
>>>>>>> if (!(UseSHA1Intrinsics || UseSHA256Intrinsics ||
>>>>>>> UseSHA512Intrinsics)) {
>>>>>>> FLAG_SET_DEFAULT(UseSHA, false); }
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 2/26/16 4:37 PM, Deshpande, Vivek R wrote:
>>>>>>>> Hi all
>>>>>>>>
>>>>>>>> I would like to contribute a patch which optimizesSHA-1
>>>>>>>> andSHA-256 for
>>>>>>>> 64 and 32 bitX86architecture using Intel SHA extensions.
>>>>>>>>
>>>>>>>> Could you please review and sponsor this patch.
>>>>>>>>
>>>>>>>> Bug-id:
>>>>>>>>
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8150767
>>>>>>>> webrev:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~vdeshpande/SHANI/8150767/webrev.00/
>>>>>>>>
>>>>>>>> Thanks and regards,
>>>>>>>>
>>>>>>>> Vivek
>>>>>>>>
>>>>>>
>>>>>
>>>
>>
>
More information about the hotspot-compiler-dev
mailing list