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

Reply via email to