RFR(M): 8131048: ppc: implement CRC32 intrinsic

[Lindenmaier, Goetz]

Anyways, a new webrev ...
http://cr.openjdk.java.net/~goetz/webrevs/8131048-crc32/webrev.03/

Best regards,
  Goetz.

-----Original Message-----
From: Lindenmaier, Goetz 
Sent: Donnerstag, 16. Juli 2015 12:20
To: 'Volker Simonis'
Cc: Vladimir Kozlov; Christian Thalinger; hotspot-dev at openjdk.java.net; Tiago Sturmer Daitx (tdaitx at br.ibm.com)
Subject: RE: RFR(M): 8131048: ppc: implement CRC32 intrinsic

HI Volker, 

> in general the change looks good. As always just a few minor comments:
Thanks!

> - I think we use C++ style '//' comments. 
The /** style comments are recently spreading in the C++ code.
Also, they are used for the CRC implementations in the other
architectures, so I would like to leave them in there.
I added the newline, though.

> This will allow you get rid of the "#if defined(ABI_ELFv2)" t the end
Done.

> Just a last general question: is this merely a plain assembler
> re-implementation of the crc32 algorithm from
> src/share/native/java/util/zip/zlib-1.2.8 ? 
Yes.
> Does it already sho any performance gains
Yes, for small input data this is faster according to Lutz. See also the RFR.
>  or do we expect these only after we use the new Power 8 instructions (something like [1])?
Yes.

Thanks,
  Goetz.

Thank you and best regards,
Volker

[1] https://github.com/antonblanchard/crc32-vpmsum


On Wed, Jul 15, 2015 at 2:01 PM, Lindenmaier, Goetz
<goetz.lindenmaier at sap.com> wrote:
> Hi Vladimir,
>
> I removed that code:
> http://cr.openjdk.java.net/~goetz/webrevs/8131048-crc32/webrev.02/
>
> Thanks for the review!
>   Goetz.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Dienstag, 14. Juli 2015 19:34
> To: Lindenmaier, Goetz; Christian Thalinger
> Cc: hotspot-dev at openjdk.java.net; Tiago Sturmer Daitx (tdaitx at br.ibm.com)
> Subject: Re: RFR(M): 8131048: ppc: implement CRC32 intrinsic
>
> I agree with separate RFE to consolidate data structures.
>
> Looks like you have experimental code left in templateInterpreter_ppc.cpp
>
> +#if 1
> +    // Performance measurements show the 1word and 2word variants to be almost equivalent,
> +    // with very light advantages for the 1word variant. We chose the 1word variant for
> +    // code compactness.
> +    __ kernel_crc32_1word(crc, data, dataLen, table, t0, t1, t2, t3, tc0, tc1, tc2, tc3);
> +#else
> +    if (UseNewCode) {
>
>
> Thanks,
> Vladimir
>
> On 7/14/15 2:01 AM, Lindenmaier, Goetz wrote:
>> Hi Christian,
>>
>> Could I first commit my change and then look into merging the tables?
>> There are three of them, aarch has one, too.  The change might take a while
>> as I would need somebody to test my aarch edits.  And I want to share this
>> change with Tiago Stuermer who eventually optimizes it for Power8.
>>
>> If I move the table to stubRoutines.cpp, the #defines from
>> stubRoutines_ppc_64.hpp would be seen there, too, and
>> I could add similar defines in the other files.  This would avoid
>> that useless data is compiled into the binary on x86 and aarch.
>>
>> Best regards,
>>    Goetz.
>>
>>
>>
>> -----Original Message-----
>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>> Sent: Montag, 13. Juli 2015 18:28
>> To: Lindenmaier, Goetz
>> Cc: hotspot-dev at openjdk.java.net; Tiago Sturmer Daitx (tdaitx at br.ibm.com)
>> Subject: Re: RFR(M): 8131048: ppc: implement CRC32 intrinsic
>>
>>
>>> On Jul 13, 2015, at 2:45 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
>>>
>>> Hi,
>>>
>>> Lutz implemented the CRC32 intrinsic for ppc.  So far, it's based on
>>> the Power7 instructions.  Power8 might allow further optimizations.
>>> Please review this change:
>>> http://cr.openjdk.java.net/~goetz/webrevs/8131048-crc32/webrev.01/
>>
>> +juint StubRoutines::ppc64::_crc_table[CRC32_TABLES][CRC32_COLUMN_SIZE] = {
>>
>> I’ aware that this table contains more than just the x86 one but I still think it would be better to share the existing table somehow:
>>
>> juint StubRoutines::x86::_crc_table[] =
>>
>>>
>>> We tested this with jck, jtreg and our benchmarks on aix, ppc64 and ppc64le.
>>> Perormance improvements are seen especially for small jobs.
>>>
>>> Best regards,
>>>   Goetz
>>>
>>>
>>