[8u] RFR for backport of 8198894 (CRC32 1/4): [PPC64] More generic vector CRC implementation (v2)

[Gustavo Romero]

Hi Martin,

On 11/06/2019 12:15 PM, Doerr, Martin wrote:
> Hi Gustavo,
> 
>> [PPC64] More generic vector CRC implementation                           (1/4)
>> http://cr.openjdk.java.net/~gromero/crc32_jdk8u/for-review/v2/8198894/
> 
> This seems to be the version I had already reviewed. I'm still ok with it.

Yes, nothing changed. I posted it again for completeness, but I see now
it can cause confusing. I'll avoid doing it in the future.


>> [PPC64] Possibly unreliable stack frame resizing in template interpreter (2/4)
>> http://cr.openjdk.java.net/~gromero/crc32_jdk8u/for-review/v2/8216376/
> 
> Normally, backports should get handled in separate backport RFRs, but the manual change in this version could be considered trivial, so I don't insist on separate RFR.
> Looks good, now.

I see, initially I posted the patches separately. I should have kept them
separated.


>> [PPC64] Vector CRC implementation should be used by interpreter and be faster for short arrays
>> http://cr.openjdk.java.net/~gromero/crc32_jdk8u/for-review/v2/8216060/   (3/4)
> 
> Almost like above. I'd leave at least the CRC32C defines in the code (stubRoutines_ppc). They don't disturb. Why should we introduce additional diffs?
Got it. I'll send a separate RFR.


>> [PPC64] Cleanup non-vector version of CRC32
>> http://cr.openjdk.java.net/~gromero/crc32_jdk8u/for-review/v2/8217459/   (4/4)
> 
> This one contains a part of another shared code change. So I can't review it as part of this RFR.
> Needs to get reviewed separately. Backport of the original change which introduced LITTLE_ENDIAN_ONLY etc. should get evaluated.

Good point, Martin. I overlooked the original change. It's a fix from Volker to
consider big-endian function descriptors when walking the stack:

https://bugs.openjdk.java.net/browse/JDK-8206173

Fix applies cleanly (except for the path adjustment). It's shared code but in
effect it's PPC64-only. I'll take care of backporting it separate.

And a nit in the test pointed out by Volker:

http://hg.openjdk.java.net/jdk/jdk/file/5bc2e9c9604d/test/hotspot/jtreg/runtime/ElfDecoder/TestElfDirectRead.java#l39

It should read "function descriptors" instead of "file descriptors", right?

I'll send a patch to jdk/jdk fixing that comment too.

Thank you!

Best regards,
Gustavo