Adding libs for OpenXL #20045

ishitaR88 · 2024-08-22T18:56:01Z

No description provided.

zl-wang · 2024-08-26T13:39:50Z

Two high-level comments:

provide a proper description of what you are trying to do in this PR;
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 · 2024-09-11T11:24:22Z

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 · 2024-09-19T18:42:22Z

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

pshipton · 2024-09-20T21:03:16Z

I tried a build with the OMR (eclipse-omr/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'

pshipton · 2024-09-20T21:06:21Z

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.

ishitaR88 · 2024-09-23T13:16:38Z

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.

pshipton · 2024-09-23T15:35:53Z

runtime/tests/redirector/jep178/module.xml

@@ -34,7 +34,7 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-ex
 			<makefilestub data="CFLAGS += -Wc,DLL,EXPORTALL">
 				<include-if condition="spec.zos_390.*"/>
 			</makefilestub>
-			<makefilestub data="UMA_CC_MODE += -qpic -brtl -bexpall">
+			<makefilestub data="UMA_CC_MODE += -fpic -brtl -bexpall">


This can't be changed unconditionally.
FYI module.xml (UMA) is mostly used for IBM Java 8 builds, but it's still (was) possible use UMA to build OpenJ9 versions, It's something we should check, if UMA is still working for Java 23 with OpenXL. By default we use cmake, but UMA can be used if adding options for debugging purposes.

runtime/cmake/platform/os/aix.cmake

runtime/cmake/platform/toolcfg/gnu.cmake

runtime/compiler/CMakeLists.txt

runtime/compiler/env/J9KnownObjectTable.cpp

keithc-ca · 2024-10-02T21:58:35Z

runtime/compiler/runtime/RelocationRecord.cpp

@@ -64,7 +64,7 @@
 #endif /* defined(J9VM_OPT_JITSERVER) */

 // TODO: move this someplace common for RuntimeAssumptions.cpp and here
-#if defined(__IBMCPP__) && !defined(AIXPPC) && !defined(LINUXPPC)
+#if (defined(__IBMCPP__) || defined(__open_xl__) && defined(__cplusplus)) && !defined(AIXPPC) && !defined(LINUXPPC)


The subexpression defined(__cplusplus) seems redundant in this C++ source file.

Fixed it in new commit

runtime/makelib/targets.mk.aix.inc.ftl

keithc-ca · 2024-10-02T22:08:39Z

runtime/makelib/targets.mk.aix.inc.ftl

@@ -178,7 +203,9 @@ $(patsubst %.s,%.o,$(filter %.s,$(UMA_FILES_TO_PREPROCESS))) : %$(UMA_DOT_O) : %

 ifdef UMA_TREAT_WARNINGS_AS_ERRORS
  ifndef UMA_SUPPRESS_WARNINGS_AS_ERRORS
-    CFLAGS += -qhalt=w
-    CXXFLAGS += -qhalt=w
+    ifneq ($(OMR_ENV_OPENXL),1)


Please invert and add equivalent options for OpenXL.

runtime/oti/j9nonbuilder.h

pshipton · 2024-10-03T18:32:21Z

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        |                                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~

keithc-ca · 2024-10-17T18:55:18Z

runtime/compiler/env/alloca_openxl.h

@@ -0,0 +1,3 @@
+#if defined(__open_xl__)


This file needs a copyright notice.

The current year should be used in the copyright notice for new files.

Changed year to 2024

zl-wang · 2024-10-21T13:55:57Z

runtime/compiler/runtime/asmprotos.h

@@ -28,7 +28,7 @@ extern "C" {
 #endif


-#if (defined(__IBMCPP__) || defined(__IBMC__) && !defined(MVS)) && !defined(J9ZOS390) && !defined(LINUXPPC64)
+#if (defined(__IBMCPP__) || defined(__IBMC__) && !defined(MVS)) && !defined(J9ZOS390) && !defined(LINUXPPC64) || defined(__open_xl__)


this looks weird, given the fact that && has higher precedence over ||. it seems dangerous or unreadable to mix up compiler macro (__open_xl__) with platform macro (J9ZOS390 and LINUXPPC64).

@r30shah this looks an existing problem as well re MVS. i think MVS is also a platform macro (i.e. not defined by the compiler. but defined to indicate the particular platform). and, I think it should be __MVS__ to begin with. if @r30shah agrees, we should fix this as well.

I agree, it should be __MVS__ and with the fix Julian suggests.

As the MVS issue is not introduced by the changes of this PR I will address this in future creating a new PR.

zl-wang · 2024-10-21T19:08:24Z

runtime/j9vm/jvm.c

@@ -4399,7 +4399,7 @@ JVM_Available(jint descriptor, jlong* bytes)
 		return JNI_FALSE;
 	}

-#if defined(WIN32) && !defined(__IBMC__)
+#if defined(WIN32) && !defined(__IBMC__) && !(defined(__open_xl__) && !defined(__cplusplus))


wondering why we need to do this to begin with. WIN32 definition already excludes the code from being picked up on AIX or zOS, what is the purpose of && with more other conditions?

zl-wang · 2024-10-21T19:17:49Z

runtime/libffi/preconf/xz64/ffi.h

@@ -96,7 +96,7 @@ extern "C" {
 #  endif
 #  ifdef _AIX /* AIX 5.1 and earlier have LONGLONG_MAX */
 #   ifndef __PPC64__
-#    if defined (__IBMC__) || defined (__IBMCPP__)
+#    if defined (__IBMC__) || defined (__IBMCPP__) || defined(__open_xl__)


for the above dozen ffi.h files on non-AIX platforms, do we really need to change this line of code, especially since it is under# ifdef _AIX to begin with. in other words, how changing or not-changing this code affect the builds on non-AIX platforms?

zl-wang · 2024-10-21T19:20:50Z

runtime/makelib/targets.mk.aix.inc.ftl

+  CFLAGS += -std=c89 -qarch=ppc -fno-strict-aliasing -fstack-protector
+else
+  CFLAGS += -qlanglvl=extended -qarch=ppc -qalias=noansi -qxflag=LTOL:LTOL0 -qsuppress=1506-1108 -qstackprotect
+endif
 CFLAGS += -D_XOPEN_SOURCE_EXTENDED=1 -D_ALL_SOURCE -DRS6000 -DAIXPPC -D_LARGE_FILES


take notice maybe: we need to revisit this flag setting in the future ... -qarch=ppc is pretty out of date and conservative.

Signed-off-by: midronij <[email protected]>

Signed-off-by: Ishita Ray <[email protected]>

Add CMAKE_C_COMPILER_IS_OPENXL macro to add the compiler flags needed by openXL17 Signed-off-by: Ishita Ray <[email protected]>

keithc-ca · 2024-11-05T14:28:16Z

runtime/j9vm/jvm.c

@@ -4542,7 +4542,7 @@ JVM_Lseek(jint descriptor, jlong bytesToSeek, jint origin)
 	}

 #if defined(WIN32)
-#ifdef __IBMC__
+#if defined(__IBMC__) || defined(__open_xl__) && !defined(__cplusplus)


This would require parentheses if __cplusplus needed to be tested, but it doesn't:

#if defined(__IBMC__) || defined(__open_xl__)

The condition should be repeated (in a comment) on #else and #endif lines.

keithc-ca · 2024-11-06T19:18:57Z

runtime/j9vm/jvm.c

@@ -4642,7 +4642,7 @@ JVM_Open(const char* filename, jint flags, jint mode)
 #define JVM_EEXIST -100

 #ifdef WIN32
-#ifdef __IBMC__
+#if defined(__IBMC__) || defined(__open_xl__) && !defined(__cplusplus)


Please remove redundant uses of defined(__cplusplus) throughout.

keithc-ca · 2024-11-06T19:19:44Z

runtime/libffi/preconf/arm/ffi.h

@@ -96,7 +96,7 @@ extern "C" {
 #  endif
 #  ifdef _AIX /* AIX 5.1 and earlier have LONGLONG_MAX */
 #   ifndef __PPC64__
-#    if defined (__IBMC__) || defined (__IBMCPP__)
+#    if defined (__IBMC__) || defined (__IBMCPP__) || defined(__open_xl__)


Please revert changes such as this (where Open XL can't apply).

keithc-ca · 2024-11-06T19:21:03Z

runtime/makelib/targets.mk.aix.inc.ftl

+  else
+    # xlc++ options
+    CXXFLAGS += -q mbcs -qinfo=pro
+  else


This is not legal (only one else is allowed).

keithc-ca · 2024-11-06T19:22:38Z

runtime/verbose/CMakeLists.txt

+if(DEFINED OMR_OS_AIX AND DEFINED CMAKE_C_COMPILER_IS_OPENXL)
+	target_link_libraries(j9vrb
+		PRIVATE


Please don't do this. If libraries are need by any compiler, they should be used for all.

keithc-ca · 2024-11-06T19:27:48Z

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").

midronij · 2024-11-27T16:17:51Z

Ishita will be away for the foreseeable future, so I will be taking over this contribution here: #20690
@zl-wang could you please close this PR when you have a moment?

Signed-off-by: midronij <[email protected]>

ishitaR88 force-pushed the openxl branch from ba9f6f8 to 68fd820 Compare August 22, 2024 19:08

ishitaR88 requested a review from dsouzai as a code owner August 22, 2024 19:16

ishitaR88 force-pushed the openxl branch from 3fdd87b to a1ca6df Compare September 11, 2024 05:40

pshipton mentioned this pull request Sep 20, 2024

OpenSSL compilation on AIX doesn't "just work" with OpenXL 7.1.2 #20207

Closed

pshipton reviewed Sep 23, 2024

View reviewed changes

ishitaR88 force-pushed the openxl branch 3 times, most recently from e426cf2 to 121bd27 Compare September 26, 2024 20:17

keithc-ca requested changes Oct 2, 2024

View reviewed changes

ishitaR88 force-pushed the openxl branch from 1d1d4e2 to fd4fd50 Compare October 3, 2024 14:20

ishitaR88 changed the title ~~Adding libs for OpenXL~~ WIP:Adding libs for OpenXL Oct 3, 2024

keithc-ca marked this pull request as draft October 3, 2024 18:34

keithc-ca changed the title ~~WIP:Adding libs for OpenXL~~ Adding libs for OpenXL Oct 3, 2024

ishitaR88 force-pushed the openxl branch from fd4fd50 to c9a7ed2 Compare October 3, 2024 18:58

ishitaR88 force-pushed the openxl branch 3 times, most recently from 0a882d4 to 60ce5cd Compare October 17, 2024 18:44

keithc-ca reviewed Oct 17, 2024

View reviewed changes

ishitaR88 force-pushed the openxl branch 2 times, most recently from 9ddb7f3 to 795bb55 Compare October 17, 2024 19:16

ishitaR88 marked this pull request as ready for review October 18, 2024 14:23

zl-wang requested changes Oct 21, 2024

View reviewed changes

midronij and others added 5 commits October 24, 2024 12:10

Add necessary libraries to linker commands

ce0f594

Signed-off-by: midronij <[email protected]>

Add __open_xl__ macro ifdefs

c2faf1f

Signed-off-by: midronij <[email protected]>

Add __open_xl__ macros to __IBMC__/__IBMCPP__ conditions

f10cda6

Signed-off-by: midronij <[email protected]>

Add __open_xl__ macro for __builtin_alloca

9ed8565

Signed-off-by: Ishita Ray <[email protected]>

Add CMAKE_C_COMPILER_IS_OPENXL macro for related flags

9301c42

Add CMAKE_C_COMPILER_IS_OPENXL macro to add the compiler flags needed by openXL17 Signed-off-by: Ishita Ray <[email protected]>

ishitaR88 force-pushed the openxl branch from 795bb55 to 9301c42 Compare October 24, 2024 16:11

keithc-ca reviewed Nov 5, 2024

View reviewed changes

Rearrange use of __open_xl__ macro

904db2c

ishitaR88 force-pushed the openxl branch from a7f4002 to 904db2c Compare November 6, 2024 14:31

keithc-ca requested changes Nov 6, 2024

View reviewed changes

ishitaR88 mentioned this pull request Nov 26, 2024

Create macro to identify openXL compiler eclipse-omr/omr#7480

Closed

midronij mentioned this pull request Nov 27, 2024

Modifications to ensure compatibility with OpenXL and xlC on AIX #20690

Merged

zl-wang closed this Nov 27, 2024

midronij added a commit to midronij/openj9 that referenced this pull request Nov 28, 2024

Requested changes from eclipse-openj9#20045

6b6cfe5

Signed-off-by: midronij <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding libs for OpenXL #20045

Adding libs for OpenXL #20045

ishitaR88 commented Aug 22, 2024

zl-wang commented Aug 26, 2024

zl-wang commented Sep 11, 2024

zl-wang commented Sep 19, 2024

pshipton commented Sep 20, 2024 •

edited

Loading

pshipton commented Sep 20, 2024

ishitaR88 commented Sep 23, 2024

pshipton Sep 23, 2024

keithc-ca Oct 2, 2024

ishitaR88 Nov 4, 2024

keithc-ca Oct 2, 2024

ishitaR88 Nov 4, 2024

pshipton commented Oct 3, 2024

keithc-ca Oct 17, 2024

ishitaR88 Oct 17, 2024

keithc-ca Nov 5, 2024

ishitaR88 Nov 6, 2024

zl-wang Oct 21, 2024 •

edited

Loading

zl-wang Oct 21, 2024

r30shah Oct 21, 2024

ishitaR88 Oct 24, 2024

zl-wang Oct 21, 2024

zl-wang Oct 21, 2024

zl-wang Oct 21, 2024

keithc-ca Nov 5, 2024

ishitaR88 Nov 6, 2024

keithc-ca Nov 6, 2024

keithc-ca Nov 6, 2024

keithc-ca Nov 6, 2024

keithc-ca Nov 6, 2024

keithc-ca commented Nov 6, 2024

midronij commented Nov 27, 2024 •

edited

Loading

Adding libs for OpenXL #20045

Adding libs for OpenXL #20045

Conversation

ishitaR88 commented Aug 22, 2024

zl-wang commented Aug 26, 2024

zl-wang commented Sep 11, 2024

zl-wang commented Sep 19, 2024

pshipton commented Sep 20, 2024 • edited Loading

pshipton commented Sep 20, 2024

ishitaR88 commented Sep 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshipton commented Oct 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zl-wang Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithc-ca commented Nov 6, 2024

midronij commented Nov 27, 2024 • edited Loading

pshipton commented Sep 20, 2024 •

edited

Loading

zl-wang Oct 21, 2024 •

edited

Loading

midronij commented Nov 27, 2024 •

edited

Loading