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

John Rose john.r.rose at oracle.com
Thu Dec 31 22:33:46 UTC 2015 

When performing explicit range checks in pre-intrinsic code,
let's try to use the new intrinsic functions in java.util.Objects,
called checkIndex, checkFromToIndex, and checkFromIndexSize.
These are simpler, safer, and more maintainable than our previous
practice of using hand-written "random logic", such as in this bug:
  http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/cb31a76eecd1#l1.52

Depending on the documented API, it is usually enough that the
thrown exception be a RuntimeException of any sort.  By default,
the methods throw a generic IndexOutOfBoundsException.
In cases where a particular exception must be thrown, the Objects
methods provide an optional "hook" for building the desired exception.

In this case, since the code is already pushed, we should clean it
up as part of this bug:
  https://bugs.openjdk.java.net/browse/JDK-8135250

— John

On Dec 29, 2015, at 9:33 AM, Kharbas, Kishor <kishor.kharbas at intel.com> wrote:
> 
> That's great.. Thank you!
> 
> I will keep the jcheck tip in mind for next time :)
> 
> - Kishor
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
> Sent: Tuesday, December 29, 2015 12:47 AM
> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
> Cc: Anthony Scarpino
> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
> 
> Hi Kishor,
> 
> There were coding style problems which I fixed. Please, do cleanup in a future (use jcheck).
> 
> src/cpu/x86/vm/stubGenerator_x86_32.cpp:2144: Trailing whitespace
> src/cpu/x86/vm/stubGenerator_x86_64.cpp:3061: Trailing whitespace
> src/cpu/x86/vm/stubRoutines_x86.hpp:36: Trailing whitespace
> src/cpu/x86/vm/vm_version_x86.cpp:709: Trailing whitespace
> src/share/vm/opto/library_call.cpp:702: Trailing whitespace
> src/share/vm/opto/runtime.hpp:317: Trailing whitespace
> 
> src/share/vm/opto/library_call.cpp:5789: Carriage return (^M)
> 
> I submitted push job. Lets see how it will go.
> 
> Regards,
> Vladimir
> 
> On 12/28/15 8:48 PM, Kharbas, Kishor wrote:
>> Vladimir, sorry that file was added accidentally.
>> Here is an updated patch - 
>> http://cr.openjdk.java.net/~vdeshpande/8143925/webrev.01/
>> 
>> This patch includes,
>> 1. Changes to some comments.
>> 2. Small correction in vm_version_x86.cpp.
>> 3. Removal of version.rc file.
>> 
>> Thanks for reviewing the code.
>> 
>> Kishor
>> 
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Thursday, December 24, 2015 4:36 PM
>> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
>> Cc: Anthony Scarpino
>> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
>> 
>> What are the changes in src/os/windows/vm/version.rc?
>> 
>> Otherwise this looks good.
>> 
>> Thanks,
>> Vladimir
>> 
>> On 12/24/15 2:26 PM, Kharbas, Kishor wrote:
>>> 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
>>>>>

More information about the hotspot-compiler-dev mailing list