RFR(S): 8218991: s390: Add intrinsic for GHASH algorithm

[Doerr, Martin]

Hi Volker and Lutz,

thanks for reviewing. Pushed.

Best regards,
Martin


-----Original Message-----
From: Volker Simonis <volker.simonis at gmail.com> 
Sent: Montag, 18. Februar 2019 11:58
To: Schmidt, Lutz <lutz.schmidt at sap.com>
Cc: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(S): 8218991: s390: Add intrinsic for GHASH algorithm

Hi Martin,

looks good!

Thanks,
Volker

On Fri, Feb 15, 2019 at 6:18 PM Schmidt, Lutz <lutz.schmidt at sap.com> wrote:
>
> Hi Martin,
> this looks good now.
> Thanks,
> Lutz
>
> On 15.02.19, 15:56, "Doerr, Martin" <martin.doerr at sap.com> wrote:
>
>     Hi Lutz,
>
>     thank you for the review.
>
>     I know you're not yet a jdk reviewer, but a qualified s390 reviewer (which is even more valuable in this case)!
>
>     Thanks for the hints. I've changed to use mvc and xc. This makes the code a little shorter.
>     (Note that I'm using the versions which use Address and do the -1 internally.)
>
>     New webrev:
>     http://cr.openjdk.java.net/~mdoerr/8218991_s390_ghash/webrev.01/
>
>     Best regards,
>     Martin
>
>
>     -----Original Message-----
>     From: Schmidt, Lutz
>     Sent: Freitag, 15. Februar 2019 10:24
>     To: Doerr, Martin <martin.doerr at sap.com>; 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
>     Subject: Re: RFR(S): 8218991: s390: Add intrinsic for GHASH algorithm
>
>     Hi Martin,
>
>     this is a nice improvement! Thanks a lot for implementing. Looks like easy harvested fruit. __
>
>     Your change looks good, overall. But remember, I’m not a reviewer.
>
>     I have two comments, though:
>     Why didn’t you use the MVC instruction when copying mem2mem? It’s available exactly for that purpose and could provide some extra ticks saved. Example:
>
>         // Copy back result and free parameter block.
>         __ z_lg( Z_R0, Address(Z_R1));
>         __ z_stg(Z_R0, Address(state));
>         __ z_lg( Z_R0, Address(Z_R1, 8));
>         __ z_stg(Z_R0, Address(state, 8));
>         __ z_aghi(Z_SP, frame_resize);
>
>     would become
>
>         // Copy back result and free parameter block.
>         __ z_mvc(0, 8-1, state, 0, R1);
>         __ z_mvc(8, 8-1, state, 8, R1);
>         __ z_aghi(Z_SP, frame_resize);
>
>     or even
>
>         // Copy back result and free parameter block.
>         __ z_mvc(0, 2*8-1, state, 0, R1);
>         __ z_aghi(Z_SP, frame_resize);
>
>     Looks pretty and compact, doesn't it? A similar transformation (two MVC instructions) is possible for "Fill parameter block".
>
>     Second: how about a
>         __ z_xc(16, 2*8-1, R1, 16, R1)
>     to remove the key from stack?
>
>     Watch out to not forget the (-1) in the len field. The instruction uses an 8-bit len field to code lengths 1..256.
>
>     Thanks,
>     Lutz
>
>
>     From: "Doerr, Martin (martin.doerr at sap.com)" <martin.doerr at sap.com>
>     Date: Thursday, 14. February 2019 at 18:28
>     To: "'hotspot-compiler-dev at openjdk.java.net'" <hotspot-compiler-dev at openjdk.java.net>
>     Cc: Lutz Schmidt <lutz.schmidt at sap.com>
>     Subject: RFR(S): 8218991: s390: Add intrinsic for GHASH algorithm
>
>     Hi,
>
>     I’d like to contribute a GHASH stub for s390 to fix a SSL performance bottleneck.
>
>     Webrev:
>     http://cr.openjdk.java.net/~mdoerr/8218991_s390_ghash/webrev.00/
>
>     TestAESMain improves by about factor 3 with the following setup on z13 hardware: algorithm=AES, mode=GCM, paddingStr=PKCS5Padding, msgSize=12000, keySize=128, noReinit=false, checkOutput=false, encInputOffset=0, encOutputOffset=0, decOutputOffset=0, lastChunkSize=32
>
>     Please review.
>
>     Best regards,
>     Martin
>
>
>
>
>