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@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@linux.vnet.ibm.com> Sent: Mittwoch, 21. November 2018 17:19 To: Kazunori Ogata <OGATAK@jp.ibm.com>; Doerr, Martin <martin.doerr@sap.com> Cc: hotspot-compiler-dev@openjdk.java.net; ppc-aix-port-dev@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@openjdk.java.net> wrote on 2018/11/19 18:12:45:
From: "Kazunori Ogata" <OGATAK@jp.ibm.com> To: "Doerr, Martin" <martin.doerr@sap.com> Cc: "hotspot-compiler-dev@openjdk.java.net" <hotspot-compiler- dev@openjdk.java.net>, "ppc-aix-port-dev@openjdk.java.net" <ppc-aix-port- dev@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@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@sap.com> To: Kazunori Ogata <OGATAK@jp.ibm.com>, "hotspot-compiler-dev@openjdk.java.net" <hotspot-compiler-dev@openjdk.java.net>, "ppc-aix-port-dev@openjdk.java.net" <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of Kazunori Ogata Sent: Montag, 19. November 2018 09:28 To: hotspot-compiler-dev@openjdk.java.net; ppc-aix-port-dev@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