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

[Christian Thalinger]

> On Feb 29, 2016, at 3:23 PM, Deshpande, Vivek R <vivek.r.deshpande at intel.com> wrote:
> 
> HI Vladimir
> 
> The thought behind using "intel" is to create generic placeholder for more functions with Intel copyright and not to restrict the file to SHA extensions.

But you can have multiple copyrights in one file.  Why doesn’t this work here?

> Let us know what you think.
> 
> Regards,
> Vivek 
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
> Sent: Monday, February 29, 2016 5:15 PM
> To: Christian Thalinger; Deshpande, Vivek R
> Cc: hotspot compiler; Rukmannagari, Shravya
> Subject: Re: RFR (M): 8150767: Update for x86 SHA Extensions enabling
> 
> 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. 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
>>>>> 
>>> 
>>