RFR (M): 8143925: Enhancing CounterMode.crypt() for AES

[Kharbas, Kishor]

Hello all,

Thank you Vladimir and Anthony for your inputs so far.
I have updated the hotspot based on the suggestions and also added CTR mode to jtreg test. 

During testing I also noticed that the Java code for CounterMode.crypt() uses the partially used encrypted counter from previous invocation and also saves the last encryptedCounter for next invocation.
This case was not handled by the intrinsic. I have fixed this in the latest patch.

Summary of changes:
1. Proper disabling of UseAESCTRIntrinsic flag based on hardware support
2. Adding the missing support explained above.
3. Added CTR mode in jtreg test 7184394
4. Added and changed some encodings (pextr and pinsr) in assembler_x86.cpp

The updated hotspot webrev is at : http://cr.openjdk.java.net/~vdeshpande/8143925/webrev.00/
There is no update to jdk webrev posted earlier which is http://cr.openjdk.java.net/~mcberg/8143925/jdk/webrev.02/
Bug id : https://bugs.openjdk.java.net/browse/JDK-8143925

Much appreciated!

Happy holidays!
Kishor


-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Friday, December 04, 2015 3:59 PM
To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
Cc: Anthony Scarpino
Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES

jdk: http://cr.openjdk.java.net/~mcberg/8143925/jdk/webrev.02/

JDK changes looks good to me.

hotspot: http://cr.openjdk.java.net/~mcberg/8143925/hotspot/webrev.04/

Please, set flag to 'false' on platforms which does not support this
intrinsic:

   if (UseAESCTRIntrinsics) {
     warning("AES/CTR intrinsics are not available on this CPU");
     FLAG_SET_DEFAULT(UseAESCTRIntrinsics, false);
   }

Also Anthony asked to add test for this intrinsic. Please do it:

"2) It would be good to add CTR to the TestAES tests.  It's in hotspot/test/compiler/codegen/7184394/.  The test currently has CBC, ECB, and GCM in it, so it should be easy.  It's also the only test I know of that tests the intrinsic.  None of the tests in the jdk repo that I know of loop enough to trigger the intrinsic."

Thanks,
Vladimir

On 12/4/15 1:40 PM, Kharbas, Kishor wrote:
> Thanks Vladimir for the feedback!
>
> I have updated the jbs entry with the new patch.
>
> JDK changes : added range checks in the JDK using additional methods.
> Hotspot changes : renamed the UseCTRAESIntrinsics flag to 
> UseAESCTRIntrinsics
>
> Further review and feedback is appreciated!
>
> - Kishor
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Tuesday, December 01, 2015 5:32 PM
> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
>
> Hotspot changes seems fine. But JDK changes should have additional method for range checks - this is new requirement for intrinsics which access arrays. See, for example, cryptBlockCheck() in AESCrypt.java.
>
> Thanks,
> Vladimir
>
> On 11/24/15 2:33 PM, Kharbas, Kishor wrote:
>> Hello all,
>>
>> I request the community to review a patch for enhancing
>> CounterMode.crypt() for AES. This patch defines intrinsic for
>> CounterMode.crypt() to leverage the parallel nature of AES in Counter
>> (CTR) Mode.
>>
>> This is achieved by operating on 6 blocks in parallel to issue 
>> independent x86 AES-NI instructions and keep the CPU pipeline full.
>>
>> Testing on micro-benchmark has shown a speedup of 4x-6x.
>>
>> Bug id:
>>
>>      https://bugs.openjdk.java.net/browse/JDK-8143925
>>
>> Webrev:
>>
>>     hotspot:
>> http://cr.openjdk.java.net/~mcberg/8143925/hotspot/webrev.02/
>>
>>     jdk: http://cr.openjdk.java.net/~mcberg/8143925/jdk/webrev.01/
>>
>> Much appreciated!
>>
>> Kishor Kharbas
>>