Ping: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"
Hiroshi H Horii
HORII at jp.ibm.com
Wed Aug 10 05:13:07 UTC 2016
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