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 >>>>> > > >>>>> > >>>>> >>> >
