RFR(M): 8217459: [PPC64] Cleanup non-vector version of CRC32

[Doerr, Martin]

Hi Gustavo,

thank you for reviewing and testing.

Seems like many comments were taken from java.util.zip.CRC32. I guess it was intended to refer to it.
I think it's not bad to have it this way because it makes it easier to compare both implementations.
Maybe Lutz can comment on this and if he would like to keep it this way.

Best regards,
Martin


-----Original Message-----
From: Gustavo Romero <gromero at linux.vnet.ibm.com> 
Sent: Mittwoch, 23. Januar 2019 23:18
To: Doerr, Martin <martin.doerr at sap.com>; 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Subject: Re: RFR(M): 8217459: [PPC64] Cleanup non-vector version of CRC32

Hi Martin,

On 01/21/2019 04:07 PM, Doerr, Martin wrote:
> PPC64 currently contains static tables for CRC32/CRC32C calculations. We only need some of them depending on Endianess and on whether vector instructions are available or not.
> We can get rid of quite some code when we generate these constants at startup as we already do for the vector version.
> In addition, we can save one register in the vector case because we can use one constants pointer for all related constants.
> Webrev:
> http://cr.openjdk.java.net/~mdoerr/8217459_ppc64_crc_consts/webrev.00/ <http://cr.openjdk.java.net/%7Emdoerr/8217459_ppc64_crc_consts/webrev.00/>

Thanks for the clean-up. Change looks good!

It's good to see fold_8bit_crc32 and kernel_crc32_1byte going away (I just
noted them recently so I missed both in my previous clean-up). And also
the static table simplification.

I tested the change with different array sizes and byte values with and
without vpmsum in the CPU, i.e. has_vpmsumb() = false, and found no issues.

Only a nit: should we update the following comment and replace 'timesXtoThe32'
by something better, maybe 'table'? That name doesn't look much meaningful in the
current context and seems taken from the native code for java.util.zip.CRC32:

3902 /**
3903  * uint32_t crc;
3904  * timesXtoThe32[crc & 0xFF] ^ (crc >> 8);
3905  */
3906 void MacroAssembler::fold_byte_crc32(Register crc, Register val, Register table, Register tmp) {


Best regards,
Gustavo