[8u] RFR for backport of JDK-8188868: PPC64: Support AES intrinsics on Big Endian

Wed Nov 21 18:58:02 UTC 2018

Hi Martin,

On 11/21/2018 02:47 PM, Doerr, Martin wrote:
> Hi Gustavo,
> 
> I think it should also be possible to backport only [1]. I just wanted to avoid that [1] gets backported now and [2] at a later point of time. It's better to do it in the right order. And I think taking [1] - [3] minimizes the manual adaptations.

Got it, thanks for the explanation.

> But don't forget that I'm not a 8u reviewer. So I'm not the person to judge about 8u backports.
> I can only state that I, as author and reviewer of the original changes, think that these backports are feasible.

Thanks for the guidance and great help as always.

> Backport requests need to get discussed on jdk8u-dev at openjdk.java.net according to http://openjdk.java.net/projects/jdk8u/enhancement-template.html

Right. On the other hand, I understand that if the patch does not apply cleanly
it must be reviewed again on the hotspot / hotspot-compile-dev ML (depending on
the original ML where the review took place) and that new review should be
mentioned on the 8u request for approval, because enhancement-template.html [1]
says that it's still necessary:

"[...] that if an enhancement is approved for fixing in JDK 8 Updates,
then a push approval request must still be submitted before the code changes are
pushed."

and http://openjdk.java.net/projects/jdk8u/approval-template.html, by its turn,
says:

"- if the review is taking place somewhere else, a link to the public review
thread. If the review from the JDK Project is relevant, a link to it should also
be included."

so I understood that doing a new 8u review on hotspot or hotspot-compile-dev ML
is not wrong, rather it's necessary when the patch does not apply cleanly.

I followed that path when I backported the CRC32 changes (with Goetz reviewing
the 8u changes on the hotspot-compiler-dev ML) and in fact I see the same flow
being followed by others on the hotspot-compiler-dev ML as well. No sure if I'm
missing something however, but if so, please let me know (you or somebody else
from the Community).

Thanks a lot.

Best regards,
Gustavo

[1] http://openjdk.java.net/projects/jdk8u/enhancement-template.html

> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Gustavo Romero <gromero at linux.vnet.ibm.com>
> Sent: Mittwoch, 21. November 2018 17:19
> To: Kazunori Ogata <OGATAK at jp.ibm.com>; Doerr, Martin <martin.doerr at sap.com>
> Cc: hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> Subject: Re: [8u] RFR for backport of JDK-8188868: PPC64: Support AES intrinsics on Big Endian
> 
> Hi Martin and Ogata,
> 
> Sorry for not replying earlier.
> 
> I'm just back from holidays and catching up with the threads.
> 
> On 11/21/2018 07:53 AM, Kazunori Ogata wrote:
>> Hi Martin,
>>
>> I noticed that the changeset JDK-8188868 [1] also needs load_perm() and
>> vec_prem(), which are added in another changeset "8185979: PPC64:
>> Implement SHA2 intrinsic" [2].  I'm sorry, but I forgot about these two
>> functions when I posted my RFR.
>>
>> The issue is that applying [2] causes a long chain of dependency, as shown
>> below in depending order, and it eventually reached two changesets [5][6]
>> that seem to be difficult to backport to JDK8.  [5] is for the compact
>> string feature, which is not supported in JDK8, and [6] is a big bundle of
>> changes.
>>
>> Is it acceptable as a back port process to cherry pick a few methods from
>> a dependent changeset, instead of applying whole changeset?
>>
>> Possible candidates of cherry picking are:
>>
>> 1. Pick load_perm() and vec_perm() from [2]
>> 2. Pick vst() and vsix() from [3], and apply [2]
>> 3. Pick has_vsx() and vsr[abst]() from [4], and apply [3] and above
>> 4. Pick has_mfdscr() from [5] and config_dscr() from [6], and apply [4]
>> and above.
>>
>> I appreciate your suggestion.
> 
> Martin, I know Ogata already said that he will pick up also the changes related
> to the SHA2 acceleration following your suggestion, but I'm still curious if it
> would be acceptable/reasonable  (assuming one would like to focus only on the
> BE AES [1] backport) to cherry pick only load_perm() and vec_perm() from [3] in
> order to backport "PPC64: Support AES intrinsics on Big Endian" [1], since both
> load_perm() and vec_perm() look rather general and simple (to maintain, handle
> in case of a merge conflict with a future backport of [3], etc).
> 
> In that case BE AES [1] backport would be simpler since JDK-8185975 [2] (needed
> by [1] as you pointed out) applies cleanly except for some offsets. vec_perm()
> is also overloaded in [1], so a slight variation of vec_perm() from [2] will be
> added anyway by [1].
> 
> Thank you.
> 
> 
> Best regards,
> Gustavo
> 
> [1] http://hg.openjdk.java.net/jdk/jdk/rev/9d337e48b178 (8188868: PPC64: Support AES intrinsics on Big Endian)
> [2] http://hg.openjdk.java.net/jdk/jdk/rev/1bf8c1e8b79a (8185975: PPC64: Fix vsldoi interface according to the ISA)
> [3] http://hg.openjdk.java.net/jdk/jdk/rev/f4962ab855b6 (8185979: PPC64: Implement SHA2 intrinsic)
> 
>>
>> [1] 8188868: PPC64: Support AES intrinsics on Big Endian
>>       http://hg.openjdk.java.net/jdk/jdk/rev/9d337e48b178
>> [2] 8185979: PPC64: Implement SHA2 intrinsic
>>       http://hg.openjdk.java.net/jdk/jdk/rev/f4962ab855b6
>> [3] 8185969: PPC64: Improve VSR support to use up to 64 registers
>>       http://hg.openjdk.java.net/jdk/jdk/rev/057f21a10f5f
>> [4] 8154156: PPC64: improve array copy stubs by using vector instructions
>>       http://hg.openjdk.java.net/jdk/jdk/rev/c9d756fa846e
>> [5] 8149655: PPC64: Implement CompactString intrinsics
>>       http://hg.openjdk.java.net/jdk/jdk/rev/6241574f5982
>> [6] 8077838: Recent developments for ppc.
>>       http://hg.openjdk.java.net/jdk/jdk/rev/c703c89fddbf
>>
>>
>> Regards,
>> Ogata
>>
>>
>> "hotspot-compiler-dev" <hotspot-compiler-dev-bounces at openjdk.java.net>
>> wrote on 2018/11/19 18:12:45:
>>
>>> From: "Kazunori Ogata" <OGATAK at jp.ibm.com>
>>> To: "Doerr, Martin" <martin.doerr at sap.com>
>>> Cc: "hotspot-compiler-dev at openjdk.java.net" <hotspot-compiler-
>>> dev at openjdk.java.net>, "ppc-aix-port-dev at openjdk.java.net"
>> <ppc-aix-port-
>>> dev at openjdk.java.net>
>>> Date: 2018/11/19 18:16
>>> Subject: RE: [8u] RFR for backport of JDK-8188868: PPC64: Support AES
>>> intrinsics on Big Endian
>>> Sent by: "hotspot-compiler-dev"
>> <hotspot-compiler-dev-bounces at openjdk.java.net>
>>>
>>> Hi Martin,
>>>
>>> Thank you for your comment.  I see.  I'll backport JDK-8185975 first.
>>>
>>>> Please don't forget to test the debug build.
>>> Yes, I'll test both debug and release builds.
>>>
>>>
>>> Regards,
>>> Ogata
>>>
>>>
>>>
>>> From:   "Doerr, Martin" <martin.doerr at sap.com>
>>> To:     Kazunori Ogata <OGATAK at jp.ibm.com>,
>>> "hotspot-compiler-dev at openjdk.java.net"
>>> <hotspot-compiler-dev at openjdk.java.net>,
>>> "ppc-aix-port-dev at openjdk.java.net" <ppc-aix-port-dev at openjdk.java.net>
>>> Date:   2018/11/19 17:56
>>> Subject:        RE: [8u] RFR for backport of JDK-8188868: PPC64: Support
>>
>>> AES intrinsics on Big Endian
>>>
>>>
>>>
>>> Hi Ogata,
>>>
>>> you need to backport JDK-8185975 before you can use +8 instead of -8.
>>> Please don't forget to test the debug build.
>>>
>>> Best regards,
>>> Martin
>>>
>>>
>>> -----Original Message-----
>>> From: hotspot-compiler-dev
>> <hotspot-compiler-dev-bounces at openjdk.java.net>
>>> On Behalf Of Kazunori Ogata
>>> Sent: Montag, 19. November 2018 09:28
>>> To: hotspot-compiler-dev at openjdk.java.net;
>>> ppc-aix-port-dev at openjdk.java.net
>>> Subject: [8u] RFR for backport of JDK-8188868: PPC64: Support AES
>>> intrinsics on Big Endian
>>>
>>> Hi,
>>>
>>> May I get reviews for enhancement backport of JDK-8188868: PPC64:
>> Support
>>> AES intrinsics on Big Endian?
>>>
>>> INVALID URI REMOVED
>>> u=http-3A__cr.openjdk.java.net_-7Ehorii_jdk8u-5Faes-5Fbe_8188868_webrev.
>>> 00_&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=p-
>>>
>> FJcrbNvnCOLkbIdmQ2tigCrcpdU77tlI2EIdaEcJw&m=BqxW_dQ87Y0TOxyOjkH6hFZIIrhwH2ycEWJdxSDcAeQ&s=ulzXDyQq40EaguNM-
>>> svdR9rWGZnClO3c3d_41XGFuHg&e=
>>>
>>>
>>> There is no code change except for an immediate parameter of vsldoi (-8
>> in
>>>
>>> jdk8u was changed to 8 to match with the latest code), besides the
>>> difference of directory tree structure.
>>>
>>> Since not a small number of customers are still using jdk8, especially
>> in
>>> AIX, JDK-8188868 is important for them to improve performance of secure
>>> network transportation.
>>>
>>>
>>> Regards,
>>> Ogata
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
> 