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

[Doerr, Martin]

Hi Gustavo,

> 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/ru
> ntime/ElfDecoder/TestElfDirectRead.java#l39
> 
> It should read "function descriptors" instead of "file descriptors", right?

Comment fixes should not be done in backports. If you would like to fix it, it should get fixed in jdk/jdk and then backported.

Please backport as it is. You won't need a review if you don't change anything except the trivial path adaptations.

Best regards,
Martin


> -----Original Message-----
> From: Gustavo Romero <gromero at linux.vnet.ibm.com>
> Sent: Donnerstag, 7. November 2019 01:54
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-
> dev at openjdk.java.net
> Cc: jdk8u-dev at openjdk.java.net
> Subject: Re: [8u] RFR for backport of 8198894 (CRC32 1/4): [PPC64] More
> generic vector CRC implementation (v2)
> 
> 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/ru
> ntime/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