RFR (M): 8150767: Update for x86 SHA Extensions enabling
Christian Thalinger
christian.thalinger at oracle.com
Wed Mar 2 22:04:49 UTC 2016
> On Mar 2, 2016, at 11:49 AM, Deshpande, Vivek R <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:
void fast_pow(XMMRegister xmm0, XMMRegister xmm1, XMMRegister xmm2, XMMRegister xmm3, XMMRegister xmm4,
XMMRegister xmm5, XMMRegister xmm6, XMMRegister xmm7, Register rax, Register rcx,
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?
>
> 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> 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-sha-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
>>>>>>>
>>>>>
>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160302/121ed255/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list