Support for AES on ppc64le
Doerr, Martin
martin.doerr at sap.com
Fri Mar 18 12:08:57 UTC 2016
Hi Hiroshi,
>> If possible, I would like to finish little endian support first.
>> In the near future, I would like to add a support of big endian.
That's ok.
I've opened a bug and created a webrev as needed for every contribution:
https://bugs.openjdk.java.net/browse/JDK-8152172
http://cr.openjdk.java.net/~mdoerr/8152172_ppc64le_aes/webrev.00/
I had to make the following changes in order to get the debug build working as well:
1. I had to remove in_bytes() which is only supported for ByteSize objects. The values are already integers.
2. The instructions like lvx don't support 0 (or r0) as second parameter (there's an assertion). We have to use the version which omits this parameter.
3. Some instructions use 4 or 5 bit signed immediates. Therefore I had to replace 4 by -4 or 16 by -16 to avoid assertions.
4. You had removed code for UseAESCTRIntrinsics, too. I have added it back since you only implemented AES but not AESCTR intrinsics.
The new version of the diff file is 8152172_ppc64le_aes.changeset which you can find in the webrev.
If this is the final version you would like to contribute, please send out a request for review with the following headline (and point to the webrev):
RFR(M): 8152172: PPC64: Support AES intrinsics
If you need additional changes, just let us know.
Best regards,
Martin
From: Hiroshi H Horii [mailto:HORII at jp.ibm.com]
Sent: Freitag, 18. März 2016 08:42
To: Doerr, Martin <martin.doerr at sap.com>
Cc: hotspot-compiler-dev at openjdk.java.net; Tim Ellison <Tim_Ellison at uk.ibm.com>; Vladimir Kozlov <vladimir.kozlov at oracle.com>; Simonis, Volker <volker.simonis at sap.com>
Subject: RE: Support for AES on ppc64le
Hi Martin and all,
Thank you for your comments.
> May we ask you to support big endian, too? AIX is still big endian.
> I think big endian linux requires setting and restoring VRSAVE which
> is not needed on little endian linux and AIX.
If possible, I would like to finish little endian support first.
In the near future, I would like to add a support of big endian.
> I have taken a look at the PPC64 part of the change and I have a
> minor change request:
> We don't like to see both strings if has_vcipher(): " vcipher" and "
> aes". Please remove one of them.
In my understanding, to pass the test of compiler/cpuflags/,
"aes" should be returned. That is, in the attached new change, "aes"
is returned instead of "vcipher".
> I noticed that you use the non-volatile vector registers v20-v31
> which may be dangerous as they are not saved & restored.
> A quick workaround to allow their usage could be to change the build
> to disallow GCC to use altivec.
> Else I think we should save & restore them in the java ENTRY_FRAME.
> I think we can assist with this.
I modified to avoid using v20-v31 in stub codes. Thank you for your correction.
> You may need additional check and cast because next expression expects
> the objAESCryptKey points to int[]:
sessionK is a private field of com.sun.crypto.provider.AESCrypt and
it always store only two int[] objects (in makeSessionKey method).
So, I believe, additional cast is not necessary to access sessionK[0] as an int[] object.
In addition, Vladimir thankfully corrected my change in load_array_element.
Please see the attached file for the detail.
Regards,
Hiroshi
-----------------------
Hiroshi Horii,
IBM Research - Tokyo
"Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote on 03/15/2016 20:57:14:
> From: "Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
> To: Hiroshi H Horii/Japan/IBM at IBMJP, Vladimir Kozlov
> <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>>
> Cc: Tim Ellison <Tim_Ellison at uk.ibm.com<mailto:Tim_Ellison at uk.ibm.com>>, "Simonis, Volker"
> <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>, "hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>"
> <hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>
> Date: 03/15/2016 20:58
> Subject: RE: Support for AES on ppc64le
>
> Hi Hiroshi,
>
> thanks for contributing AES support. We appreciate it.
>
> May we ask you to support big endian, too? AIX is still big endian.
> I think big endian linux requires setting and restoring VRSAVE which
> is not needed on little endian linux and AIX.
>
> I have taken a look at the PPC64 part of the change and I have a
> minor change request:
> We don't like to see both strings if has_vcipher(): " vcipher" and "
> aes". Please remove one of them.
>
> I noticed that you use the non-volatile vector registers v20-v31
> which may be dangerous as they are not saved & restored.
> A quick workaround to allow their usage could be to change the build
> to disallow GCC to use altivec.
> Else I think we should save & restore them in the java ENTRY_FRAME.
> I think we can assist with this.
>
> Thanks and best regards,
> Martin
>
> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> bounces at openjdk.java.net<mailto:bounces at openjdk.java.net>] On Behalf Of Hiroshi H Horii
> Sent: Dienstag, 15. März 2016 10:31
> To: Vladimir Kozlov <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>>
> Cc: Tim Ellison <Tim_Ellison at uk.ibm.com<mailto:Tim_Ellison at uk.ibm.com>>; Simonis, Volker
> <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>; hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: Support for AES on ppc64le
>
> Hi Vladimir,
>
> Thank you a lots for your quick response and review.
>
> To use load_array_element, SEGV happened, I needed the following change.
> Could you also review this change is reasonable?
>
> diff --git a/src/share/vm/opto/graphKit.cpp b/src/share/vm/opto/graphKit.cpp
> --- a/src/share/vm/opto/graphKit.cpp
> +++ b/src/share/vm/opto/graphKit.cpp
> @@ -1680,6 +1680,8 @@
> Node* GraphKit::load_array_element(Node* ctl, Node* ary, Node* idx,
> const TypeAryPtr* arytype) {
> const Type* elemtype = arytype->elem();
> BasicType elembt = elemtype->array_element_basic_type();
> + if (elembt == T_NARROWOOP)
> + elembt = T_OBJECT;
> Node* adr = array_element_address(ary, idx, elembt, arytype->size());
> Node* ld = make_load(ctl, adr, elemtype, elembt, arytype,
> MemNode::unordered);
> return ld;
>
> I attached a full diff that is applied your kind suggestions.
>
>
>
> Regards,
> Hiroshi
> -----------------------
> Hiroshi Horii, Ph.D.
> IBM Research - Tokyo
>
>
> Vladimir Kozlov <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>> wrote on 03/15/2016 05:24:49:
>
> > From: Vladimir Kozlov <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>>
> > To: Hiroshi H Horii/Japan/IBM at IBMJP, hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
> > Cc: "Simonis, Volker" <volker.simonis at sap.com<mailto:volker.simonis at sap.com>>, Tim Ellison
> > <Tim_Ellison at uk.ibm.com<mailto:Tim_Ellison at uk.ibm.com>>
> > Date: 03/15/2016 05:25
> > Subject: Re: Support for AES on ppc64le
> >
> > Hi Hiroshi
> >
> > About library_call.cpp changes.
> >
> > You don't need GraphKit::
> >
> > And you can use load_array_element() instead:
> >
> > Node* objAESCryptKey = load_array_element(control(), objSessionK,
> > intcon(0), TypeAryPtr::OOPS);
> >
> > You may need additional check and cast because next expression expects
> > the objAESCryptKey points to int[]:
> >
> > Node* k_start = array_element_address(objAESCryptKey, intcon(0), T_INT);
> >
> > Thanks,
> > Vladimir
> >
> > On 3/14/16 9:34 AM, Hiroshi H Horii wrote:
> > > Dear all:
> > >
> > > Can I please request reviews for the following change?
> > > This change was created for JDK 9.
> > >
> > > Description:
> > > This change adds stub routines support for single-block AES encryption and
> > > decryption operations on the POWER8 platform. They are availableonly when
> > > the application is configured to use SunJCE crypto provider on little
> > > endian.
> > > These stubs make use of efficient hardware AES instructions and thus
> > > offer significant performance improvements over JITed code on POWER8
> > > as on x86 and SPARC. AES stub routines are enabled by default on POWER8
> > > platforms that support AES instructions (vcipher). They can be
> > > explicitly enabled or
> > > disabled on the command-line using UseAES and UseAESIntrinsics JVM flags.
> > > Unlike x86 and SPARC, vcipher and vnchiper of POWER8 need the same round
> > > keys of AES. Therefore, inline_aescrypt_Block in library_call.cpp calls
> > > the stub with
> > > AESCrypt.sessionK[0] as round keys.
> > >
> > > Summary of source code changes:
> > >
> > > *src/cpu/ppc/vm/assembler_ppc.hpp
> > > *src/cpu/ppc/vm/assembler_ppc.inline.hpp
> > > - Adds support for vrld instruction to rotate vector register values
> > > with
> > > left doubleword.
> > >
> > > *src/cpu/ppc/vm/stubGenerator_ppc.cpp
> > > - Defines stubs for single-block AES encryption and
> decryption routines
> > > supporting all key sizes (128-bit, 192-bit and 256-bit).
> > > - Current POWER AES decryption instructions are not compatible with
> > > SunJCE expanded decryption key format. Thus decryption stubs read
> > > the expanded encryption keys (sessionK[0]) with descendant order.
> > > - Encryption stubs use SunJCE expanded encryption key as their is
> > > no incompatibility issue between POWER8 AES encryption instructions
> > > and SunJCE expanded encryption keys.
> > >
> > > *src/cpu/ppc/vm/vm_version_ppc.cpp
> > > - Detects AES capabilities of the underlying CPU by using
> has_vcipher().
> > > - Enables UseAES and UseAESIntrinsics flags if the underlying CPU
> > > supports AES instructions and neither of them is explicitly
> > > disabled on
> > > the command-line. Generate warning message if either of these
> > > flags are
> > > enabled on the command-line whereas the underlying CPU does not
> > > support
> > > AES instructions.
> > >
> > > *src/share/vm/opto/library_call.cpp
> > > - Passes the first input parameter, reference to sessionK[0] to the
> > > AES stubs
> > > only on the POWER platform.
> > >
> > > Code change:
> > > Please see an attached diff file that was generated with "hg diff
> > > -g" under
> > > the latest hotspot directory.
> > >
> > > Passed tests:
> > > jtreg compiler/codegen/7184394/
> > > jtreg compiler/cpuflags/ (after removing @ignored annotation)
> > >
> > > * This is my first post of a change. I'm sorry in advance if I don't
> > > follow the
> > > community manners.
> > >
> > > * I wrote this description based on the follows.
> > > http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2013-
> > November/012670.html
> > >
> > >
> > >
> > > Regards,
> > > Hiroshi
> > > -----------------------
> > > Hiroshi Horii,
> > > IBM Research - Tokyo
> > >
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160318/e4073959/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list