RFR: 8073108: GHASH Intrinsics

[Anthony Scarpino]

On 02/16/2015 06:01 PM, Vladimir Kozlov wrote:
> On 2/16/15 5:23 PM, David Holmes wrote:
>> Hi Tony,
>>
>> Not a review as hotspot compiler folk need to review this.
>>
>> On 17/02/2015 7:11 AM, Anthony Scarpino wrote:
>>> Hi,
>>>
>>> I'm requesting a code review to intrinsify the GHASH operations for both
>>> x86 and SPARC platforms.  This greatly increases performance over
>>> software for AES/GCM crypto operations, details are in the bug.
>>
>> This may be normal for intrinsics but there seems to be a large amount
>> of shared code being updated to support these platform specific
>> enhancements. What happens on other platforms if the user sets
>> UseGHASHIntrinsics? Shouldn't there be a guard against this in
>> arguments.cpp?
>
> Code in vm_version_<arch>.cpp does the check. vm_version_sparc.cpp and
> vm_version_x86.cpp do that.
>
> Tony, please, fix other vm_version_<arch>.cpp files (including closed)
> to set corresponding flag to false. See UseAES as example.

Ok.. thanks Dave & Vladimir for bring that up.. I fix that..


> Also code in vm_version_sparc.cpp should be similar to one in
> vm_version_x86.cpp. Something like:
>
>    // GHASH/GCM intrinsics
>    if (has_vis3() && (UseVIS > 2)) {
>      if (FLAG_IS_DEFAULT(UseGHASHIntrinsics)) {
>        UseGHASHIntrinsics = true;
>      }
>    } else if (UseGHASHIntrinsics) {
>      if (!FLAG_IS_DEFAULT(UseGHASHIntrinsics))
>        warning("GHASH intrinsics require VIS3 insructions support.
> Intriniscs will be disabled");
>      FLAG_SET_DEFAULT(UseGHASHIntrinsics, false);
>    }
>

Sure..

> There is #ifdef _WIN64 in stubGenerator_x86_32.cpp. It is not needed
> since it is 32-bit code.

Sure.. Do I still need to save the xmm6 and xmm7 on 32bit code, or is 
all of the register saving only for Windows 64bit?

>
> I see you switched off -DcheckOutput=false for GCM testing and return
> from compareArrays() after length compare. Is it because it can't be
> done or you did not have time to add needed code?

I'll have to double check, but I believe -DcheckOutput can be turned 
back on.  I suspect I never updated the @run lines after I changed 
compareArrays()

> Otherwise Hotspot code looks good.
>
> Thanks,
> Vladimir
>
>>
>> Thanks,
>> David
>>
>>> The review is for two repos, hotspot and jdk:
>>>
>>> http://cr.openjdk.java.net/~ascarpino/8073108/hotspot/webrev/
>>>
>>> http://cr.openjdk.java.net/~ascarpino/8073108/jdk/webrev/
>>>
>>> thanks
>>>
>>> Tony