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

OJDK MH Support for JDK8 #14937

Merged
merged 3 commits into from
May 25, 2022
Merged

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Apr 20, 2022

  1. Disable VarHandle specific OJDK MH code in Java 8

    Based upon current support, VarHandle specifc code should only be
    enabled for JDK11 and onwards. Enabling it for JDK8 leads to the
    VarHandle class not-found errors.

  2. MethodHandleResolver changes to support OJDK MHs in JDK8

    In JDK8, ...

    1. MethodHandleNatives.linkCallSite does not accept indexInCP as the second
      parameter. This should resolve a compilation error.
    2. resolveInvokeDynamic should throw the original error wrapped in a
      BootstrapMethodError if method-type evaluation, from the descriptor string,
      fails.
    3. Testing indicates that inaccessibleTypeCheck is not needed in JDK8, and
      it also leads to compilation errors. So, this check has been removed from
      resolveInvokeDynamic and sendResolveMethodHandle.
  3. MHN changes to support OJDK MHs in JDK8

    1. MethodHandleNatives.resolve has a different signature. Account for
      behaviour changes across JDK8, 11 and 17+.
    2. Assume speculativeResolve to be false in MethodHandleNatives.resolve
      if it is not included in the parameter list.
    3. MethodHandleNatives.getConstant only exists for JDK8. It returns false
      suggesting that OpenJ9 does not support the corresponding feature.
    4. MethodHandleNatives.copyOutBootstrapArguments only exists in JDK11+.
    5. MethodHandleNatives.clearCallSiteContext only exists in JDK11+.
    6. Spread long single lines across multiple lines.

Closes: #14556

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

@babsingh
Copy link
Contributor Author

babsingh commented May 2, 2022

@fengxue-IS Can you please review this PR?

@babsingh babsingh requested a review from fengxue-IS May 2, 2022 16:36
@babsingh
Copy link
Contributor Author

babsingh commented May 2, 2022

All the pending JDK8 OJDK-MH failures, which will be seen after merging this PR, are linked to #14541.

Copy link
Contributor

@fengxue-IS fengxue-IS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes lgtm

one minor format pick, plus maybe remove the Unsupported error as we will never be in that path (given that anything pre-Java 17 except 8&11 is out of support)

runtime/oti/jclprots.h Outdated Show resolved Hide resolved
runtime/oti/jclprots.h Outdated Show resolved Hide resolved
Based upon current support, VarHandle specifc code should only be
enabled for JDK11 and onwards. Enabling it for JDK8 leads to the
VarHandle class not-found errors.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh babsingh added the project:MH Used to track Method Handles related work label May 20, 2022
In JDK8, ...
1. MethodHandleNatives.linkCallSite does not accept indexInCP as the second
parameter. This should resolve a compilation error.
2. resolveInvokeDynamic should throw the original error wrapped in a
BootstrapMethodError if method-type evaluation, from the descriptor string,
fails.
3. Testing indicates that inaccessibleTypeCheck is not needed in JDK8, and
it also leads to compilation errors. So, this check has been removed from
resolveInvokeDynamic and sendResolveMethodHandle.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
1. MethodHandleNatives.resolve has a different signature. Account for
   behaviour changes across JDK8, 11 and 17+.
2. Assume speculativeResolve to be false in MethodHandleNatives.resolve
   if it is not included in the parameter list.
3. MethodHandleNatives.getConstant only exists for JDK8. It returns false
   suggesting that OpenJ9 does not support the corresponding feature.
4. MethodHandleNatives.copyOutBootstrapArguments only exists in JDK11+.
5. MethodHandleNatives.clearCallSiteContext only exists in JDK11+.
6. Spread long single lines across multiple lines.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@tajila
Copy link
Contributor

tajila commented May 25, 2022

Jenkins test sanity win jdk8

@tajila
Copy link
Contributor

tajila commented May 25, 2022

Jenkins test sanity plinux jdk17

@tajila tajila merged commit afc8574 into eclipse-openj9:master May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project:MH Used to track Method Handles related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[JDK8 OJDK-MH] Build Errors
4 participants