RFR(M) 8180823: [s390] Rework/optimize AES intrinsics
lutz.schmidt at sap.com
Mon Jul 31 15:43:00 UTC 2017
Thanks for the review. Please find my comments re. your findings below, marked with “>>>”. The changes are reflected in a new webrev iteration at http://cr.openjdk.java.net/~lucy/webrevs/8180823.01/
On 31.07.2017, 15:28, "Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
nice change. Thanks for also fixing the stack pointer update order (missing part of 8180659). I didn’t find real bugs, but I’d like to get at least some comments fixed.
* The comment in front of generate_push_Block mentions a “JIT_TIMER” timer which you didn’t contribute.
>>> You are right. JIT_TIMER is a SAP internal tool. Comment removed.
* The comments describing the stack layout claim that there were requirements “min. size AES_parmBlk_addspace” and “Z_SP after expansion, octoword-aligned”. I think neither the minimal size nor the octoword alignment of the SP are required, so please remove these comments if this is correct. I believe that only the parmBlk needs the alignment.
>>> Comments are fixed.
* The stack reservation is a little oversized. Maximum space used by alignment is AES_parmBlk_align-8. Please fix at least the comment. The spill space is also oversized (only 2 slots are used). I leave it up to you if you want to reduce stack space reservation. As this frame is only on top while processing AES stuff, it’s not really an issue to waste a few bytes.
>>> Add’l (spill) space requirement is adapted (3*8 bytes). I would like to keep the alignment stuff “as is”.
* generate_AES_cipherBlock contains an unnecessary rotate_then_insert instruction which seems to be a leftover from some removed functionality.
>>> removed. Was a JIT_TIMER leftover which wasn’t correctly annotated.
I can also sponsor this change after we go a 2nd review.
From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Schmidt, Lutz
Sent: Freitag, 28. Juli 2017 13:06
To: hotspot-compiler-dev at openjdk.java.net
Subject: RFR(M) 8180823: [s390] Rework/optimize AES intrinsics
I would like to request reviews for this s390-only enhancement:
The code for AES intrinsics in stubGenerator_s390.cpp is reworked to get rid of multiple copies of basically the same code snippets. Some chances for minor performance improvements are also taken advantage of.
The code has been active locally @SAP for several weeks now. No problems popped up.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-compiler-dev