openj9 icon indicating copy to clipboard operation
openj9 copied to clipboard

Adding libs for OpenXL

Open ishitaR88 opened this issue 1 year ago • 7 comments

ishitaR88 avatar Aug 22 '24 18:08 ishitaR88

Two high-level comments:

  1. provide a proper description of what you are trying to do in this PR;
  2. you cannot modify the code as you did, since it breaks the builds using xlC compiler. keep in mind that the codebase feeds into java8/11/17/21/newer builds. we only intended to switch to OpenXL for 'newer' builds like java23. if you change the code as you did in this PR, java8-21 cannot be built anymore (with xlC). you have to change the code (and makefile etc) conditionally to be compatible with both compilers.

zl-wang avatar Aug 26 '24 13:08 zl-wang

please indicate if it cleared the criteria that, with these changes in place, we can build with both xlC/xlclang and OpenXL (also staying or better the performance). that is when it is ready to be reviewed.

zl-wang avatar Sep 11 '24 11:09 zl-wang

did I miss your changes to makefiles, to set CC and CXX? at least, i haven't seen them yet.

zl-wang avatar Sep 19 '24 18:09 zl-wang

I tried a build with the OMR (https://github.com/eclipse/omr/pull/7447) and these OpenJ9 changes. https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK23_ppc64_aix_Personal/37 I added the compiler path (only works on paix822) and defined CC and CXX https://github.ibm.com/Peter-Shipton/tooling/tree/openxl I had to fix (hack) openssl.gmk as well as revert the change to use xlc https://github.com/pshipton/openj9-openjdk-jdk23/tree/openxl

It got this far:

16:49:15  [ 41%] Running preprocessing p/runtime/ebb.spp to create /home/jenkins/workspace/Build_JDK23_ppc64_aix_Personal/build/aix-ppc64-server-release/vm/runtime/compiler/p/runtime/ebb.ipp
16:49:15  error: invalid value 'extc99' in '-std=extc99'

pshipton avatar Sep 20 '24 21:09 pshipton

did I miss your changes to makefiles, to set CC and CXX?

defaults.yml should be updated similarly to https://github.ibm.com/Peter-Shipton/tooling/commit/eebd2986b8a3b

It won't work on any OpenJ9 open machine until we update them to have the 17.1.2 compiler.

pshipton avatar Sep 20 '24 21:09 pshipton

I tried a build with the OMR (eclipse/omr#7447) and these OpenJ9 changes. https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK23_ppc64_aix_Personal/37 I added the compiler path (only works on paix822) and defined CC and CXX https://github.ibm.com/Peter-Shipton/tooling/tree/openxl I had to fix (hack) openssl.gmk as well as revert the change to use xlc https://github.com/pshipton/openj9-openjdk-jdk23/tree/openxl

It got this far:

16:49:15  [ 41%] Running preprocessing p/runtime/ebb.spp to create /home/jenkins/workspace/Build_JDK23_ppc64_aix_Personal/build/aix-ppc64-server-release/vm/runtime/compiler/p/runtime/ebb.ipp
16:49:15  error: invalid value 'extc99' in '-std=extc99'

Yes there was an issue. I have fixed that.will push the modification shortly.

ishitaR88 avatar Sep 23 '24 13:09 ishitaR88

Tried a new build https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK23_ppc64_aix_Personal/52 There are a number of errors like

14:27:22  /home/jenkins/workspace/Build_JDK23_ppc64_aix_Personal/openj9/runtime/compiler/optimizer/InterpreterEmulator.cpp:1594:105: error: too many arguments to function call, expected 0, have 1
14:27:22   1594 |                                                       (int32_t) _currentCallMethod->virtualCallSelector(cpIndex), cpIndex, _currentCallMethod,
14:27:22        |                                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~

pshipton avatar Oct 03 '24 18:10 pshipton

The description is incomplete. This isn't just about libraries - many command-line options are adjusted and source is changed.

Please follow the commit guidelines at https://github.com/eclipse-omr/omr/blob/master/CONTRIBUTING.md#commit-guidelines (the commit summary and the headline here should be "written in the imperative mood").

keithc-ca avatar Nov 06 '24 19:11 keithc-ca

Ishita will be away for the foreseeable future, so I will be taking over this contribution here: https://github.com/eclipse-openj9/openj9/pull/20690 @zl-wang could you please close this PR when you have a moment?

midronij avatar Nov 27 '24 16:11 midronij