That's fine. Thanks, Volker
On Wed, Mar 30, 2016 at 6:43 PM, Vladimir Kozlov <vladimir.koz...@oracle.com> wrote: > BTW, I am changing UseAES check in stubs generation with UseAESIntrinsics > check as we do for other intrinsics. > > Thanks, > Vladimir > > > On 3/30/16 9:39 AM, Vladimir Kozlov wrote: >> >> Looks good. I will run hotspot compiler pre-integration testing and let >> you know results. >> >> Thanks, >> Vladimir >> >> On 3/30/16 3:29 AM, Volker Simonis wrote: >>> >>> Hi everybody, >>> >>> here finally comes the updated version of Hiroshi's change (sorry for >>> the delay): >>> >>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172.hs/ >>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172.jdk/ >>> >>> With regard to the previous version it contains three small updates: >>> - changed the type of AESCrypt::sessionK from Object[] to int[][] >>> - also updated the type of the AESCrypt::sessionK field to "[[I" in >>> LibraryCallKit::get_key_start_from_aescrypt_object() where it gets >>> loaded. >>> - protected the generation of the AES-stubs by "if (UseAES)" tp >>> prevent them from beeing generated on ppc64 big endian. >>> >>> We need a sponsor to push these two changes in sync to hs-comp. >>> >>> Thank you and best regards, >>> Volker >>> >>> >>> On Tue, Mar 29, 2016 at 10:58 PM, Anthony Scarpino >>> <anthony.scarp...@oracle.com> wrote: >>>> >>>> Volker, >>>> >>>> Changing this with your other changes in hs-comp repo is best.. Just let >>>> me >>>> know when you are ready for code review on this piece. >>>> >>>> thanks >>>> >>>> Tony >>>> >>>> >>>> >>>> >>>> On 03/29/2016 06:05 AM, Volker Simonis wrote: >>>>> >>>>> >>>>> Hi Anthony, Vladimir, >>>>> >>>>> thanks for your evaluation. Should I open a new bug for the change in >>>>> AESCrypt or can we do it under the same bug id like the ppc-specific >>>>> AES intrinisc (i.e. "8152172: PPC64: Support AES intrinsics") ? >>>>> >>>>> I also think that the AESCrypt::sessionK type change change, although >>>>> it is a class library change, should be done right in hs-comp together >>>>> with the new ppc64 AES intrinsics. Do you agree? >>>>> >>>>> Regards, >>>>> Volker >>>>> >>>>> >>>>> On Mon, Mar 28, 2016 at 7:41 PM, Anthony Scarpino >>>>> <anthony.scarp...@oracle.com> wrote: >>>>>> >>>>>> >>>>>> [Switching to security-dev, core-lib-dev was bcc'ed] >>>>>> >>>>>> I don't see any problem changing it to int[][]. Being an Object[] is >>>>>> odd >>>>>> in >>>>>> my opinion. >>>>>> >>>>>> Tony >>>>>> >>>>>> >>>>>> On 03/25/2016 04:00 PM, Vladimir Kozlov wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> This question is for core libs. >>>>>>> >>>>>>> For JIT to generate optimal code we want to change type of >>>>>>> AESCrypt::sessionK from Object[] to int[][]. >>>>>>> Otherwise JIT have to generate checkcast code to make sure that >>>>>>> elements >>>>>>> of sessionK array are int[]. >>>>>>> I see that sessionK field is private so we are not changing public >>>>>>> API. >>>>>>> >>>>>>> Thanks, >>>>>>> Vladimir >>>>>>> >>>>>>> On 3/24/16 6:17 PM, Hiroshi H Horii wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Hi Vladimir, >>>>>>>> >>>>>>>> Thank you for your reviewing. >>>>>>>> >>>>>>>> Is it possible to modify the type of sessionK from Object[] to >>>>>>>> int[][]? >>>>>>>> >>>>>>>> We can guess, the type was designed for flexibility. However, only >>>>>>>> int[] is used >>>>>>>> to store elements of sessionK, so far. In addition, this field is >>>>>>>> private. >>>>>>>> Though adding checkcast is another solution, it produces overheads. >>>>>>>> >>>>>>>> I attached an additional patch (generated with hg diff -g) to jdk >>>>>>>> directory. >>>>>>>> I would like to ask thoughts about this change of java code. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Regards, >>>>>>>> Hiroshi >>>>>>>> ----------------------- >>>>>>>> Hiroshi Horii, >>>>>>>> IBM Research - Tokyo >>>>>>>> >>>>>>>> >>>>>>>> Vladimir Kozlov <vladimir.koz...@oracle.com> wrote on 03/24/2016 >>>>>>>> 09:09:11: >>>>>>>> >>>>>>>> > From: Vladimir Kozlov <vladimir.koz...@oracle.com> >>>>>>>> > To: Hiroshi H Horii/Japan/IBM@IBMJP, "hotspot-compiler- >>>>>>>> > d...@openjdk.java.net" <hotspot-compiler-...@openjdk.java.net> >>>>>>>> > Cc: "Doerr, Martin" <martin.do...@sap.com>, Tim Ellison >>>>>>>> > <tim_elli...@uk.ibm.com>, "Simonis, Volker" >>>>>>>> <volker.simo...@sap.com> >>>>>>>> > Date: 03/24/2016 09:19 >>>>>>>> > Subject: Re: RFR(M): 8152172: PPC64: Support AES intrinsics >>>>>>>> > >>>>>>>> > I think we need to add gen_checkcast for objAESCryptKey node >>>>>>>> since >>>>>>>> > java code has no guarantee that sessionK array elements are >>>>>>>> > initialized with int[]. Or we should modify java code to >>>>>>>> declare >>>>>>>> sessionK >>>>>>>> > as int[][]. >>>>>>>> > >>>>>>>> > Thanks, >>>>>>>> > Vladimir >>>>>>>> > >>>>>>>> > Note, intrinsics are correctly handle case when >>>>>>>> > On 3/22/16 8:47 AM, Hiroshi H Horii wrote: >>>>>>>> > > Dear all: >>>>>>>> > > >>>>>>>> > > Can I please request reviews for the following change? >>>>>>>> > > This change was created for JDK 9 to enable POWER8's AES >>>>>>>> > > instructions for AES calculation. >>>>>>>> > > >>>>>>>> > > This request follows this discussion. >>>>>>>> > > >>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016- >>>>>>>> > March/021926.html >>>>>>>> > > >>>>>>>> > > Description: >>>>>>>> > > This change adds stub routines support for single-block AES >>>>>>>> > > encryption and decryption operations on the POWER8 platform. >>>>>>>> > > They are available only 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, 192 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. >>>>>>>> > > >>>>>>>> > > *src/share/vm/opto/graphKit.cpp >>>>>>>> > > - Supports T_NARROWOOP type for >>>>>>>> GraphKit::load_array_element. >>>>>>>> > > >>>>>>>> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8152172 >>>>>>>> > > Webrev: >>>>>>>> http://cr.openjdk.java.net/~mdoerr/8152172_ppc64le_aes/webrev.00/ >>>>>>>> > > >>>>>>>> > > >>>>>>>> > > Regards, >>>>>>>> > > Hiroshi >>>>>>>> > > ----------------------- >>>>>>>> > > Hiroshi Horii, >>>>>>>> > > IBM Research - Tokyo >>>>>>>> > > >>>>>>>> > >>>>>>>> >>>>>> >>>> >
