RFR(m) 8164920: ppc: enhancement of CRC32 intrinsic

[Hiroshi H Horii]

Hi Martin, and all
(Please allow me to send this mail twice. The first mail is awaiting 
because it exceeded 100KB)

Thank you for your reviewing. Gustavo and I recreated a new change based 
on your comments. I would like to request a review again.

My account of cr server is not available now (because of my mistake...) 
and Gustavo cannot create a webrev file with another reason. I would like 
to attach a diff file created with "hg diff -g" in hotspot.  If possible, 
could someone create a webrev file with this changeset?



Regards,
Hiroshi
-----------------------
Hiroshi Horii, Ph.D.
IBM Research - Tokyo

"Doerr, Martin" <martin.doerr at sap.com> wrote on 09/13/2016 18:35:09:

> From: "Doerr, Martin" <martin.doerr at sap.com>
> To: Hiroshi H Horii/Japan/IBM at IBMJP, "hotspot-compiler-
> dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.java.net>
> Cc: "Volker Simonis (volker.simonis at gmail.com)" 
> <volker.simonis at gmail.com>, Gustavo Bueno Romero <gromero at br.ibm.com>
> Date: 09/13/2016 18:36
> Subject: RE: RFR(m) 8164920: ppc: enhancement of CRC32 intrinsic
> 
> Hi Hiroshi,
> 
> we appreciate your change. Thanks for contributing it.
> It basically looks good, but I’d like to propose some minor 
improvements.
> 
> 
> kernel_crc32_1word_vpmsumd:
> 
> 1. The Pre-align code can be implemented shorter:
>     clrldi_(prealign, buf, 57);
>     beq(CCR0, L_alignHead);
> 
>     subfic(prealign, prealign, 128);
> 
> 2. I'd prefer the label name “L_alignedHead”.
> 
> 3. The branch b(L_alignTail) and the label are not needed and should
> get removed.
> 
> 
> kernel_crc32_1word_aligned:
> 
> 1. When saving and restoring non-volatile vector register, please 
> use offset differences of -16 instead of -32.
> (The ABI allows up to 288 bytes to be used in frameless functions so
> it will fit if -16 is used.)
> 
> 2. The std instructions should better be used with int offsets so 
> you can get rid of the addi(offset, offset, -8) instructions.
> 
> 
> Comments:
> For single line comments "//" should be used instead of "/*". Would 
> be nice if you could change them.
> 
> 
> Thanks and best regards,
> Martin
> 
> 
> From: Hiroshi H Horii [mailto:HORII at jp.ibm.com] 
> Sent: Dienstag, 6. September 2016 16:50
> To: hotspot-compiler-dev at openjdk.java.net; vladimir.kozlov at oracle.com
> Cc: Volker Simonis (volker.simonis at gmail.com) 
> <volker.simonis at gmail.com>; Doerr, Martin <martin.doerr at sap.com>; 
> Gustavo Bueno Romero <gromero at br.ibm.com>
> Subject: RFR(m) 8164920: ppc: enhancement of CRC32 intrinsic
> 
> Dear Vladimir and all:
> 
> Can I please request reviews for the following change?
> 
> JIRA: https://bugs.openjdk.java.net/browse/JDK-8164920
> webrev: http://cr.openjdk.java.net/~gromero/8164920/01/
> 
> As Volker's comments in the above JIRA, this is a ppc64-only 
> improvement which will not 
> affect any of the Oracle platforms in any way. 
> 
> This change includes new implementation of CRC32 Intrinsics for ppc64le.
> In my local experiment, CRC32 of 64KB was calculated more than 20 
> times faster than original. 
> Performance of CRC32 Intrinsic is important to run recent Apache 
Cassandra.
> A Cassandra daemon needs to read 64KB data from a disk with CRC32 
> checksum by default.
> 
> This JIRA entry has "jdk9-fc-request" label.
> If there is a chance to include new change in JDK 9 for ppc64le, I 
> would like to request 
> a review for this change.
> 
> Regards,
> Hiroshi
> -----------------------
> Hiroshi Horii, Ph.D.
> IBM Research - Tokyo

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160918/179889f0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hotspot.crc32.20160918.changeset
Type: application/octet-stream
Size: 60026 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160918/179889f0/hotspot.crc32.20160918-0001.changeset>