Ping: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"
Zoltán Majó
zoltan.majo at oracle.com
Wed Aug 10 13:09:30 UTC 2016
Hi,
On 08/10/2016 10:19 AM, Sean Coffey wrote:
> 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 :)
thank you, I started the push job.
Best regards,
Zoltan
>>
>> 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