RFR(M): 8144847: PPC64: Update Transactional Memory and Atomic::cmpxchg code

[Lindenmaier, Goetz]

Hi Martin,

thanks for doing this, looks good.
I’ll sponsor it.

Best regards,
  Goetz.

From: Doerr, Martin
Sent: Donnerstag, 10. Dezember 2015 16:07
To: Thomas Stüfe <thomas.stuefe at gmail.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Cc: hotspot-compiler-dev at openjdk.java.net
Subject: RE: RFR(M): 8144847: PPC64: Update Transactional Memory and Atomic::cmpxchg code

Hi,

this new webrev applies to hs-rt:
http://cr.openjdk.java.net/~mdoerr/8144847_ppc_updates/webrev.01/

It only touches PPC64 files.

I have made the changes requested by Thomas.
I only had to remove a minor interpreter variable name change. The remainder fits to hs-rt.

Please have a look.

Best regards,
  Martin


From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
Sent: Mittwoch, 9. Dezember 2015 08:28
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(M): 8144847: PPC64: Update Transactional Memory and Atomic::cmpxchg code

Hi Martin,

You could split the os kernel detection from the RTM change and submit the former to hs-rt now.

Kind regards, Thomas

On Tue, Dec 8, 2015 at 3:08 PM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
Hi Thomas,

thanks for the hint. There are changes in hs-comp and hs-rt which would cause trouble with my change at the moment. I’ll wait until they get merged and create a new webrev which hopefully applies to both repositories.

Best regards,
  Martin

From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>]
Sent: Dienstag, 8. Dezember 2015 09:22
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(M): 8144847: PPC64: Update Transactional Memory and Atomic::cmpxchg code

Hi Martin,

thanks for this addition :)

It may make a lot of sense to rebase this change to hs-rt, because os_aix.cpp is quite different there after http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/rev/ce87b1141c12. Otherwise we may have problems later applying your change atop of my change.

-------------

About the AIX kernel version recognition: I know we talked about this, but I have second thoughts now. I guess I did not think it really through before, sorry.
So, now I have a change request:

Instead of introducing os::Aix::os_kernel_version (version,release,techlevel,sp) beside the already existing os::Aix::os_version (version,release) I would prefer just one parameter, os_version, end enriching this by techlevel and sp. So, exactly what you did for os_kernel_version.

Basically, as a prototype:

// -1 = uninitialized, otherwise 32 bit number:
//  0xVVRRTTSS
//  VV - major version
//  RR - minor version
//  TT - tech level, if known, 0 otherwise
//  SS - service pack, if known, 0 otherwise
static uint32_t os_version ();

Then please change the few users of os::Aix::os_version() to now expect a 32bit unsigned number. As far as I see there are only 3 callsites.

-------------------

Other small nitpicks:

- in libodm_aix.cpp, please use trcVerbose() instead of if (Verbose) tty->.. . Please include misc_aix.hpp for trcVerbose(). We will change all those tracecalls to Unified logging in the near future and this would help me finding all trace occurrences.

- please move ~dynamicOdm() and odmWrapper::clean_wrapper() from libodm_aix.hpp to libodm_aix.cpp and accordingly remove the includes dlfcn.h and stdlib.h from libodm_aix.hpp.

- I probably would change "static unsigned int determine_os_kernel_version(int major_aix_version, int minor_aix_version);" to " "static bool fill_in_os_kernel_version(unsigned int* p_os_version);", but that is just a matter of taste.

Kind Regards, Thomas


On Mon, Dec 7, 2015 at 6:10 PM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
Hi,

I have created a webrev for further PPC64 updates:
AIX supports Transactional Memory with a certain kernel patch level. Add a detection for it and make UseRTMLocking usable on AIX.
In addition, implement Atomic::cmpxchg for jbyte.

The webrev is here:
http://cr.openjdk.java.net/~mdoerr/8144847_ppc_updates/webrev.00/

Please review.

Best regards,
  Martin



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151211/cca26833/attachment-0001.html>