Skip to content
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

[FFI] Mixed code in the JDK21 extension repo on Power #17884

Closed
ChengJin01 opened this issue Jul 28, 2023 · 8 comments
Closed

[FFI] Mixed code in the JDK21 extension repo on Power #17884

ChengJin01 opened this issue Jul 28, 2023 · 8 comments
Labels
jdk21 project:panama Used to track Project Panama related work

Comments

@ChengJin01
Copy link

ChengJin01 commented Jul 28, 2023

I notice that https://github.com/ibmruntimes/openj9-openjdk-jdk21/tree/openj9/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64 are currently merged with the Hostpot specific code intended for pLinux as follows:
/linux
ABIv2CallArranger.java
CallArranger.java
PPC64Architecture.java
TypeClass.java

which is wrong as these code are specific to the Hotspot implementation which doesn't belong the openj9 branch.

If we keep them there, it is easy to mess things up in compilation with such confusion.

FYI: @tajila, @pshipton

@ChengJin01 ChengJin01 changed the title Mixed code in JDK21 on Power [FFI]Mixed code in the JDK21 extension repo on Power Jul 28, 2023
@ChengJin01 ChengJin01 changed the title [FFI]Mixed code in the JDK21 extension repo on Power [FFI] Mixed code in the JDK21 extension repo on Power Jul 28, 2023
@ChengJin01 ChengJin01 added project:panama Used to track Project Panama related work jdk21 labels Jul 28, 2023
@ChengJin01
Copy link
Author

ChengJin01 commented Jul 28, 2023

@tajila & @keithc-ca,

I am considering to remove these Hotspot specific files/folders from https://github.com/ibmruntimes/openj9-openjdk-jdk21/tree/openj9/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64 to avoid any issue on our side if you have no objection to this.

@keithc-ca
Copy link
Contributor

In what way is the code specific to hotspot? I would expect the implementation to respect the ABI of the target platform which should be independent of any JVM implementation.

@ChengJin01
Copy link
Author

Technically, we don't use any of these code there from OpenJ9 perspective as they are totally different in implementation. Keeping them there is misleading & easy to make mistakes in the future given our own code in the OpenJDK extension is only linked to our own FFI implementation.

@ChengJin01
Copy link
Author

I also notice that https://github.com/ibmruntimes/openj9-openjdk-jdk21/blob/openj9/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java has been changed to point to the FFI implementation in OpenJ9 when merging the latest master branch.

@ChengJin01
Copy link
Author

ChengJin01 commented Jul 28, 2023

As discussed with @keithc-ca offline, it is reasonable to keep these generic code in OpenJDK to avoid any conflicts from upstream, in which case I will need to figure out a way to keep both AIX and pLinux (from OpenJ9 perspective) work correctly with all unnecessary code removed.

@ChengJin01
Copy link
Author

I will get back to work on this issue later after my vacation next week.

ChengJin01 pushed a commit to ChengJin01/openj9-openjdk-jdk21 that referenced this issue Aug 14, 2023
The changes aim to unify the linker related code on AIX
and pLinux by leveraging the latest FFI specific code on
pLinux in OpenJDK to avoid any conflicts from upstream
in the future.

Fixes: #eclipse-openj9/openj9/issues/17884

Signed-off-by: ChengJin01 <[email protected]>
@ChengJin01
Copy link
Author

With my solution at ChengJin01/openj9-openjdk-jdk21@9680612, it works good as expected on Power (AIX & pLinux) by verifying the FFI specific test suites in OpenJ9/Jtreg.

ChengJin01 pushed a commit to ChengJin01/openj9-openjdk-jdk21 that referenced this issue Aug 14, 2023
The changes aim to unify the linker related code on AIX
and pLinux by leveraging the latest FFI specific code on
pLinux in OpenJDK to avoid any conflicts from upstream
in the future.

Fixes: #eclipse-openj9/openj9/issues/17884

Signed-off-by: ChengJin01 <[email protected]>
ChengJin01 pushed a commit to ChengJin01/openj9-openjdk-jdk21 that referenced this issue Aug 14, 2023
The changes aim to unify the linker related code on AIX
and pLinux by leveraging the latest FFI specific code on
pLinux in OpenJDK to avoid any conflicts from upstream
in the future.

Fixes: #eclipse-openj9/openj9/issues/17884

Signed-off-by: ChengJin01 <[email protected]>
@ChengJin01
Copy link
Author

Close this issue as all related PRs are merged to the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jdk21 project:panama Used to track Project Panama related work
Projects
None yet
Development

No branches or pull requests

2 participants