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.

Sign up for GitHub

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

Jump to bottom

Add SVM validations for JITServer ResolvedHandle/DynamicMethod APIs #20875

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Add SVM validations for JITServer ResolvedHandle/DynamicMethod APIs #20875

merged 1 commit into from
Jan 2, 2025

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Dec 31, 2024 •
edited
Loading

The JITServer version of getResolvedDynamicMethod and getResolvedHandleMethod were missing SVM validations, which caused errors when enabling AOT support for Method Handles with JITServer.

@dsouzai dsouzai added comp:jitserver Artifacts related to JIT-as-a-Service project comp:jit:aot project:MH Used to track Method Handles related work labels Dec 31, 2024
@dsouzai
Copy link
Contributor Author

dsouzai commented Dec 31, 2024

@mpirvu could you please review?

@mpirvu mpirvu self-assigned this Dec 31, 2024
mpirvu
mpirvu approved these changes Dec 31, 2024
View reviewed changes
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented Dec 31, 2024

Conflicts must be resolved before starting testing.

@dsouzai
Add SVM validations for JITServer ResolvedHandle/DynamicMethod APIs
58b5f4d 
The JITServer version of getResolvedDynamicMethod and
getResolvedHandleMethod were missing SVM validations, which caused
errors when generate AOT support for Method Handles with JITServer.

Signed-off-by: Irwin D'Souza <[email protected]>
@mpirvu
Copy link
Contributor

mpirvu commented Dec 31, 2024

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit,alinux64jit jdk21

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 2, 2025 •
edited
Loading

All failing tests look like this:

...
[2024-12-31T20:14:44.846Z] FAILED test targets:
[2024-12-31T20:14:44.846Z] 	jdk_vector_float128_j9_0 - Test results: passed: 1 
[2024-12-31T20:14:44.846Z] 		Failed test cases: 
...

Looking at the actual test output:

[2024-12-31T19:50:14.690Z] Test results: passed: 1
[2024-12-31T19:50:21.315Z] Report written to /home/jenkins/workspace/Test_openjdk21_j9_sanity.openjdk_aarch64_linux_jit_Personal_testList_1/aqa-tests/TKG/output_17356717461274/jdk_vector_float128_j9_0/report/html/report.html
[2024-12-31T19:50:21.315Z] Results written to /home/jenkins/workspace/Test_openjdk21_j9_sanity.openjdk_aarch64_linux_jit_Personal_testList_1/aqa-tests/TKG/output_17356717461274/jdk_vector_float128_j9_0/work
[2024-12-31T19:50:21.315Z] grep #VECTOR API /home/jenkins/workspace/Test_openjdk21_j9_sanity.openjdk_aarch64_linux_jit_Personal_testList_1/aqa-tests/TKG/../TKG/output_17356717461274/jdk_vector_float128_j9_0/work/jdk/incubator/vector/Float128VectorTests.jtr
[2024-12-31T19:50:21.315Z] -----------------------------------
[2024-12-31T19:50:21.315Z] jdk_vector_float128_j9_0_FAILED
[2024-12-31T19:50:21.315Z] -----------------------------------

Not really sure why the test is failing, but either way it doesn't look related to this change since the change is AOT related and AOT MH is not enabled by default.

@mpirvu
Copy link
Contributor

mpirvu commented Jan 2, 2025

Not really sure why the test is failing

All the VectorAPI tests are currently failing with JITServer because the verbose options are global and are not passed to the server. This is a known issue and needs to be fixed by changing the tests to look at JIT compilation logs and not at vlogs. vlogs are mostly for runtime diagnostics and not for optimizer diagnostics.

@mpirvu
Copy link
Contributor

mpirvu commented Jan 2, 2025

Since all failures are known, this PR is ready to be merged.

@mpirvu mpirvu merged commit fee2f8a into eclipse-openj9:master Jan 2, 2025
10 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpirvu mpirvu mpirvu approved these changes

Assignees

@mpirvu mpirvu

Labels
comp:jit:aot comp:jitserver Artifacts related to JIT-as-a-Service project project:MH Used to track Method Handles related work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dsouzai @mpirvu