RFR (M): 8150767: Update for x86 SHA Extensions enabling

[Deshpande, Vivek R]

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