Ping: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"
Volker Simonis
volker.simonis at gmail.com
Wed Aug 10 07:40:43 UTC 2016
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