RFR(M) 8180823: [s390] Rework/optimize AES intrinsics

[Doerr, Martin]

Pushed with adapted error message and updated copyright.

Best regards,
Martin

From: Schmidt, Lutz
Sent: Dienstag, 1. August 2017 15:06
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(M) 8180823: [s390] Rework/optimize AES intrinsics

Thanks, Goetz, for having a look.

Regards, Lutz

On 01.08.2017, 14:58, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>> wrote:

Hi Lutz,

I had a look at your change. It looks good. I ran the aes
jtreg tests on it which passed.
I’m not completely happy with the error message in vm_version,
as it differs from that on x86 with the same flag settings. I think
similar behavior of the platforms is desirable. But the string
seems not to be tested, thus does no harm to the tests.

Best regards,
  Goetz.

From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Schmidt, Lutz
Sent: Montag, 31. Juli 2017 17:43
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>; hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(M) 8180823: [s390] Rework/optimize AES intrinsics

Hi Martin,

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/

Regards, Lutz


On 31.07.2017, 15:28, "Doerr, Martin" <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:

Hi Lutz,

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.

Best regards,
Martin


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<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: RFR(M) 8180823: [s390] Rework/optimize AES intrinsics

Dear all,

I would like to request reviews for this s390-only enhancement:
Bug:    https://bugs.openjdk.java.net/browse/JDK-8180823
Webrev: http://cr.openjdk.java.net/~lucy/webrevs/8180823.00/

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.

Thank you!
Lutz


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20170801/efecd2f5/attachment-0001.html>