-
Notifications
You must be signed in to change notification settings - Fork 727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modifications to ensure compatibility with OpenXL and xlC on AIX #20690
base: master
Are you sure you want to change the base?
Conversation
The OMR PR eclipse-omr/omr#7561 |
@zl-wang here is the new OpenXL PR for OpenJ9. Marked as a WIP since I'm still addressing the remaining review comments from Ishita's PR |
8661a7b
to
6b6cfe5
Compare
It's @keithc-ca who should take a look, probably most of the comments were from him. |
I didn't see anything obvious. We'll need to check it doesn't break xlc, both on jenkins and vmfarm, if that hasn't been done yet. |
To ensure that OpenJ9 can be built on AIX with both OpenXL and xlC, make modifications to compiler flags, macros, and linked libraries. Signed-off-by: midronij <[email protected]>
6b6cfe5
to
eac524c
Compare
I've confirmed that these changes don't break the xlC build on either jenkins or vmfarm. However, I did run into some issues when trying to do an OpenXL build on jenkins. Namely, when building DDR this error message popped up:
(source: https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK23_ppc64_aix_Personal/61/console) I'm able to bypass this temporarily by adding
(source: https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK23_ppc64_aix_Personal/62/console) I've run into errors like this when building locally, and was able to deal with them by adding additional libraries to the linker commands, but as far as I can tell, the makefile that I would need to modify to deal with this one in particular isn't part of OpenJ9 or OMR:
|
I believe we have 17.1.1.2 as per https://github.ibm.com/runtimes/infrastructure/issues/7704, but current issue is https://github.ibm.com/runtimes/infrastructure/issues/9368 |
i have these on my machine:
and, shr2_64.o is indeed in
it might be due to your OpenXL ND (non default) installation, such that libraries were not installed under the usual directory ( |
On the build machine
|
I tried building again with
(source: https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK23_ppc64_aix_Personal/63/console) |
you need to add one config option to indicate to the linker where to look up the runtime libraries (instead of the default location on AIX machine ... i.e. /usr/lib or /usr/lib64). peter already got you that location: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few questions since some of these changes impact platforms that are not AIX+OPENXL.
if(NOT OMR_OS_OSX) | ||
# This flag isn't supported on OSX and on AIX while using openxl compiler, | ||
# but it has this behaviour by default. | ||
if(NOT OMR_OS_OSX AND NOT OMR_OS_AIX AND NOT CMAKE_C_COMPILER_IS_OPENXL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think as this stands, this will also remove the flag for AIX when not using openxl. If you want to something like:
!(OSX || (AIX && OPENXL))
that expands (via a series of DeMorgan's) to
!OSX && (!AIX || !OPENXL)
target_link_libraries(j9gcchk | ||
PRIVATE | ||
j9vm_interface | ||
j9util | ||
j9utilcore | ||
j9avl | ||
j9hashtable | ||
j9thr | ||
j9stackmap | ||
j9pool | ||
j9hookable | ||
j9modronstartup | ||
|
||
#TODO do we just want to link against j9gc? | ||
j9gcbase | ||
j9gcstructs | ||
j9gcstats | ||
j9gccheck | ||
omrgc | ||
) | ||
endif() | ||
#TODO do we just want to link against j9gc? | ||
j9gcbase | ||
j9gcstructs | ||
j9gcstats | ||
j9gccheck | ||
omrgc | ||
j9gctrc | ||
j9gctrcstandard | ||
j9gctrcvlhgc | ||
j9gc | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to change what happens when !defined(DEFINED OMR_OS_AIX AND DEFINED CMAKE_C_COMPILER_IS_OPENXL)
; why are we modifying non AIX+OPENXL platforms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the original intent was to limit this to AIX and OpenXL, but that condition was removed as requested on Ishita's old PR: #20045 (comment)
target_link_libraries(j9gcchk_full | ||
PRIVATE | ||
j9vm_interface | ||
j9util | ||
j9utilcore | ||
j9avl | ||
j9hashtable | ||
j9thr | ||
j9stackmap | ||
j9pool | ||
j9hookable | ||
j9modronstartup | ||
|
||
#TODO do we just want to link against j9gc? | ||
j9gcbase_full | ||
j9gcstructs_full | ||
j9gcstats_full | ||
j9gccheck_full | ||
omrgc_full | ||
) | ||
endif() | ||
#TODO do we just want to link against j9gc? | ||
j9gcbase_full | ||
j9gcstructs_full | ||
j9gcstats_full | ||
j9gccheck_full | ||
omrgc_full | ||
j9gctrc_full | ||
j9gctrcstandard_full | ||
j9gctrcvlhgc_full | ||
j9gc_full | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above.
@@ -35,6 +35,7 @@ target_link_libraries(constgen | |||
j9vm_interface | |||
omrport | |||
j9thr | |||
j9util |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the implications of this for non AIX+OPENXL?
CXXFLAGS += -qxlcompatmacros -fno-rtti -fno-exceptions | ||
CXXFLAGS += -qxlcompatmacros -fno-rtti -fno-exceptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeated options.
j9gc | ||
j9gctrc | ||
j9gcapi | ||
j9gctrcstandard | ||
j9gctrcvlhgc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the implications of this for non AIX+OPENXL?
j9gc | ||
j9gctrc | ||
j9gcapi | ||
j9gctrcstandard | ||
j9gctrcvlhgc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the implications of this for non AIX+OPENXL?
@@ -137,6 +137,7 @@ SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0-only WITH Classpath-ex | |||
<library name="j9modronstartup"/> | |||
<library name="omrgcstartup" type="external"/> | |||
<library name="omrgcverbose" type="external"/> | |||
<library name="j9gc"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the implications of this for non AIX+OPENXL?
Modify compiler flags, macros, and linked libraries to ensure that OpenJ9 can be built with both OpenXL and xlC on AIX.
Updated version of #20045