Support for AES on ppc64le
Hiroshi H Horii
HORII at jp.ibm.com
Tue Mar 22 13:37:49 UTC 2016
Hi Martin,
Thank you for your modification of my codes and creating webrev and bug
entries.
Your modification is reasonable. I confirmed my local test codes work well
with it.
I would like to send a request out for a review.
Regards,
Hiroshi
-----------------------
Hiroshi Horii,
IBM Research - Tokyo
"Doerr, Martin" <martin.doerr at sap.com> wrote on 03/18/2016 21:08:57:
> From: "Doerr, Martin" <martin.doerr at sap.com>
> To: Hiroshi H Horii/Japan/IBM at IBMJP
> Cc: "hotspot-compiler-dev at openjdk.java.net" <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>
> Date: 03/18/2016 21:10
> Subject: RE: Support for AES on ppc64le
>
> 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> wrote on 03/15/2016 20:57:14:
>
> > From: "Doerr, Martin" <martin.doerr at sap.com>
> > To: Hiroshi H Horii/Japan/IBM at IBMJP, Vladimir Kozlov
> > <vladimir.kozlov at oracle.com>
> > Cc: Tim Ellison <Tim_Ellison at uk.ibm.com>, "Simonis, Volker"
> > <volker.simonis at sap.com>, "hotspot-compiler-dev at openjdk.java.net"
> > <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] On Behalf Of Hiroshi H Horii
> > Sent: Dienstag, 15. März 2016 10:31
> > To: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> > Cc: Tim Ellison <Tim_Ellison at uk.ibm.com>; Simonis, Volker
> > <volker.simonis at sap.com>; 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> wrote on 03/15/2016
05:24:49:
> >
> > > From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> > > To: Hiroshi H Horii/Japan/IBM at IBMJP,
hotspot-compiler-dev at openjdk.java.net
> > > Cc: "Simonis, Volker" <volker.simonis at sap.com>, Tim Ellison
> > > <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 UseAESIntrinsicsJVM
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/20160322/3222a771/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list