Ping: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"
Sean Coffey
sean.coffey at oracle.com
Wed Aug 10 08:19:07 UTC 2016
Thanks all. Approved for jdk8u-dev.
regards,
Sean.
On 10/08/2016 08:40, Volker Simonis wrote:
> Hi Hiroshi,
>
> thanks a lot for checking!
>
> @Zoltan: so we're ready to go :)
>
> Thanks,
> Volker
>
>
> On Wed, Aug 10, 2016 at 7:13 AM, Hiroshi H Horii <HORII at jp.ibm.com> wrote:
>> Hi Volker,
>>
>> I confirmed this AES Intrinsics improve SPECjbb2013 scores (MAXjOPs) 7-8%.
>> Thank you for your great work.
>>
>> Regards,
>> Hiroshi
>> -----------------------
>> Hiroshi Horii, Ph.D.
>> IBM Research - Tokyo
>>
>>
>>
>>
>> From: Volker Simonis <volker.simonis at gmail.com>
>> To: Zoltán Majó <zoltan.majo at oracle.com>, Hiroshi H
>> Horii/Japan/IBM at IBMJP
>> Cc: Seán Coffey <sean.coffey at oracle.com>,
>> "jdk8u-dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>, HotSpot Open
>> Source Developers <hotspot-dev at openjdk.java.net>
>> Date: 08/10/2016 00:49
>> Subject: Re: Ping: [8u] request for approval: "8152172: PPC64:
>> Support AES intrinsics"
>> ________________________________
>>
>>
>>
>> On Tue, Aug 9, 2016 at 11:03 AM, Zoltán Majó <zoltan.majo at oracle.com> wrote:
>>> Hi Volker,
>>>
>>>
>>> On 08/08/2016 03:43 PM, Volker Simonis wrote:
>>>> Ping...
>>>>
>>>> Can I please get a review for this downport. The shared changes all
>>>> cleanly apply to jdk8, only the ppc64 part required a small tweak (see
>>>> intial mail).
>>>
>>> you changes look good for me. I can sponsor the changes, if you want.
>>>
>> Hi Zoltan,
>>
>> thanks a lot. That would be really great!
>> I'm just waiting for the final confirmation from Hiroshi. One he gives
>> his OK, it would be really nice if you could sponsor the change.
>>
>> Thanks again,
>> Volker
>>
>>> Thank you and best regards,
>>>
>>>
>>> Zoltan
>>>
>>>
>>>> Thanks,
>>>> Volker
>>>>
>>>> On Wed, Aug 3, 2016 at 1:32 PM, Seán Coffey <sean.coffey at oracle.com>
>>>> wrote:
>>>>> On 02/08/16 20:47, Volker Simonis wrote:
>>>>>> Hi Sean,
>>>>>>
>>>>>> thanks a lot for your reply.
>>>>>>
>>>>>> You're right - I'm still waiting for a review of the hotspot part. Any
>>>>>> volunteers :)
>>>>> OK - would be good to get a hotspot reviewer to look at those first.
>>>>>>
>>>>>> Regarding the noreg label: I used 'noreg-other' because there already
>>>>>> exist AES tests (they have never been part of this original change).
>>>>>> Is that the right label? There are simply too many different noreg
>>>>>> labels without documentation so if I selected the wrong one, please
>>>>>> advice which one to choose.
>>>>> noreg labels are documented at :
>>>>> http://openjdk.java.net/guide/changePlanning.html#noreg
>>>>> noreg-other seems fine. You might want to add a short comment to bug
>>>>> outlining your comment on existing AES tests.
>>>>>
>>>>> regards,
>>>>> Sean.
>>>>>
>>>>>> Thanks,
>>>>>> Volker
>>>>>>
>>>>>>
>>>>>> On Tue, Aug 2, 2016 at 9:58 AM, Seán Coffey <sean.coffey at oracle.com>
>>>>>> wrote:
>>>>>>> Volker,
>>>>>>>
>>>>>>> Have the jdk8u hotspot edits been reviewed ? Looks like they should
>>>>>>> be.
>>>>>>>
>>>>>>> Can you add a suitable noreg label to the master bug report also.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Sean.
>>>>>>>
>>>>>>>
>>>>>>> On 29/07/2016 19:58, Volker Simonis wrote:
>>>>>>>> Ping!
>>>>>>>>
>>>>>>>> Can I please have an approval for backporting this change to 8u-dev?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Volker
>>>>>>>>
>>>>>>>>
>>>>>>>> On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
>>>>>>>> <volker.simonis at gmail.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> could you please approve the backport of the following, mostly ppc64
>>>>>>>>> change to jdk8u-dev:
>>>>>>>>>
>>>>>>>>> 8152172: PPC64: Support AES intrinsics
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
>>>>>>>>> Webrev
>>>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
>>>>>>>>> Review:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
>>>>>>>>> URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
>>>>>>>>> URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f
>>>>>>>>>
>>>>>>>>> As you can see, the change consists of two parts - the main one in
>>>>>>>>> the
>>>>>>>>> hotpsot repo and a small part in the jdk repo.
>>>>>>>>>
>>>>>>>>> The jdk part applied cleanly to jdk8u (with the usual directory
>>>>>>>>> shuffling).
>>>>>>>>>
>>>>>>>>> The hotspot part required a small tweak, but only in the ppc-only
>>>>>>>>> part. This is because the feature detection for the AES instructions
>>>>>>>>> was already active in jdk9 when the original change was made but is
>>>>>>>>> not available in jdk8u until now. You can find the additional
>>>>>>>>> changes
>>>>>>>>> at the end of this mail for your convenience.
>>>>>>>>>
>>>>>>>>> The required shared changes cleanly apply to jdk8u.
>>>>>>>>>
>>>>>>>>> As Vladimir pointed out in a previous thread, shared Hotspot changes
>>>>>>>>> have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
>>>>>>>>> that and synchronously push both parts.
>>>>>>>>>
>>>>>>>>> @Hiroshii: could you please verify that you are fine with the
>>>>>>>>> change?
>>>>>>>>> I've done some tests on Power8LE but just want to be sure this
>>>>>>>>> downport satisfies your needs as well.
>>>>>>>>>
>>>>>>>>> Thank you and best regards,
>>>>>>>>> Volker
>>>>>>>>>
>>>>>>>>> =====================
>>>>>>>>>
>>>>>>>>> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
>>>>>>>>> --- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016
>>>>>>>>> -0700
>>>>>>>>> +++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016
>>>>>>>>> +0200
>>>>>>>>> @@ -452,6 +476,7 @@
>>>>>>>>> a->popcntw(R7, R5); // code[7] ->
>>>>>>>>> popcntw
>>>>>>>>> a->fcfids(F3, F4); // code[8] ->
>>>>>>>>> fcfids
>>>>>>>>> a->vand(VR0, VR0, VR0); // code[9] ->
>>>>>>>>> vand
>>>>>>>>> + a->vcipher(VR0, VR1, VR2); // code[10] ->
>>>>>>>>> vcipher
>>>>>>>>> a->blr();
>>>>>>>>>
>>>>>>>>> // Emit function to set one cache line to zero. Emit function
>>>>>>>>> descriptor and get pointer to it.
>>>>>>>>> @@ -495,6 +520,7 @@
>>>>>>>>> if (code[feature_cntr++]) features |= popcntw_m;
>>>>>>>>> if (code[feature_cntr++]) features |= fcfids_m;
>>>>>>>>> if (code[feature_cntr++]) features |= vand_m;
>>>>>>>>> + if (code[feature_cntr++]) features |= vcipher_m;
>>>>>>>>>
>>>>>>>>> // Print the detection code.
>>>>>>>>> if (PrintAssembly) {
>>>>>>>>> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
>>>>>>>>> --- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016
>>>>>>>>> -0700
>>>>>>>>> +++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016
>>>>>>>>> +0200
>>>>>>>>> @@ -42,6 +42,7 @@
>>>>>>>>> fcfids,
>>>>>>>>> vand,
>>>>>>>>> dcba,
>>>>>>>>> + vcipher,
>>>>>>>>> num_features // last entry to count features
>>>>>>>>> };
>>>>>>>>> enum Feature_Flag_Set {
>>>>>>>>> @@ -56,6 +57,7 @@
>>>>>>>>> fcfids_m = (1 << fcfids ),
>>>>>>>>> vand_m = (1 << vand ),
>>>>>>>>> dcba_m = (1 << dcba ),
>>>>>>>>> + vcipher_m = (1 << vcipher),
>>>>>>>>> all_features_m = -1
>>>>>>>>> };
>>>>>>>>> static int _features;
>>>>>>>>> @@ -83,6 +85,7 @@
>>>>>>>>> static bool has_fcfids() { return (_features & fcfids_m) !=
>>>>>>>>> 0;
>>>>>>>>> }
>>>>>>>>> static bool has_vand() { return (_features & vand_m) != 0;
>>>>>>>>> }
>>>>>>>>> static bool has_dcba() { return (_features & dcba_m) != 0;
>>>>>>>>> }
>>>>>>>>> + static bool has_vcipher() { return (_features & vcipher_m) != 0;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> static const char* cpu_features() { return _features_str; }
>>>>>>>
>>
>>
>>
More information about the jdk8u-dev
mailing list