Skip to content

Fold Invokers.checkVarHandleGenericType using const refs#23639

Merged
mpirvu merged 1 commit into
eclipse-openj9:masterfrom
matthewhall2:fold_vh_checkGenericType
Jun 19, 2026
Merged

Fold Invokers.checkVarHandleGenericType using const refs#23639
mpirvu merged 1 commit into
eclipse-openj9:masterfrom
matthewhall2:fold_vh_checkGenericType

Conversation

@matthewhall2

Copy link
Copy Markdown
Contributor
  • Modifies getMethodHandleTableEntryIndex to use the asTypeCache when there is no exact MethodType match and the cache is non-null.
  • folds the call to a const ref load when the KOI is not UNKNOWN

@matthewhall2 matthewhall2 requested a review from dsouzai as a code owner April 2, 2026 14:50
@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch 3 times, most recently from 9eadcd7 to 6a72ea1 Compare April 2, 2026 16:44
@matthewhall2 matthewhall2 requested a review from nbhuiyan April 2, 2026 16:45
Comment thread runtime/compiler/env/VMJ9.cpp Outdated
Comment thread runtime/compiler/env/VMJ9.cpp Outdated
Comment thread runtime/compiler/optimizer/MethodHandleTransformer.cpp Outdated
@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch from 6a72ea1 to 0b27535 Compare April 14, 2026 19:29
@matthewhall2 matthewhall2 requested a review from nbhuiyan April 14, 2026 19:31

@nbhuiyan nbhuiyan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other than the few minor comments, LGTM.

Comment thread runtime/compiler/env/VMJ9.cpp
Comment thread runtime/compiler/optimizer/MethodHandleTransformer.cpp Outdated
Comment thread runtime/compiler/optimizer/MethodHandleTransformer.cpp
Comment thread runtime/compiler/optimizer/MethodHandleTransformer.cpp
@matthewhall2

Copy link
Copy Markdown
Contributor Author

@0xdaryl can you run the tests with eclipse-omr/omr#8209 ?

@0xdaryl

0xdaryl commented May 12, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity all jdk21 depends eclipse-omr/omr#8209

@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch from 0b27535 to e09d7af Compare May 19, 2026 19:58
@matthewhall2

Copy link
Copy Markdown
Contributor Author

windows sanity.functional fails look related to #22758 (comment)

windows sanity.openjdk fail was caused by the assertion fail "orphaned const ref", I've pushed the fix for that
@0xdaryl can you run the tests again please?

@0xdaryl

0xdaryl commented May 19, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity all jdk21 depends eclipse-omr/omr#8209

@0xdaryl

0xdaryl commented May 20, 2026

Copy link
Copy Markdown
Contributor

You may need to rebase this and we try again.

@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch from e09d7af to 6f0b6c4 Compare May 21, 2026 13:42
@matthewhall2

Copy link
Copy Markdown
Contributor Author

rebased

@0xdaryl

0xdaryl commented May 21, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity xlinux,plinux,alinux,zlinux jdk21 depends eclipse-omr/omr#8209

Starting small, with just the Linux platforms first.

@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch 2 times, most recently from a114640 to f7b44cc Compare June 2, 2026 19:06
@matthewhall2

Copy link
Copy Markdown
Contributor Author

Z failure is from the vector API tests, which Ehsan confirmed was fixed. I did rebase OMR but I guess I need to again.
P failure looks unrelated (ACTION: testng -- Error. Agent communication error: java.net.SocketException: Broken pipe) - hopefully is won't fail after the rebase

@mpirvu mpirvu removed the request for review from dsouzai June 4, 2026 17:44
@matthewhall2

Copy link
Copy Markdown
Contributor Author

@mpirvu or @nbhuiyan can you run the tests again please?

@mpirvu mpirvu self-assigned this Jun 11, 2026
@mpirvu

mpirvu commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity xlinux,plinux,alinux,zlinux jdk21 depends eclipse-omr/omr#8209

@nbhuiyan

Copy link
Copy Markdown
Member

@matthewhall2 I had a few comments in my approval (approved with comments instead of requesting changes as it was just prior to me going on vacation) that have not been addressed yet.

@nbhuiyan nbhuiyan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please review the last set of comments.

@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch 2 times, most recently from 647f284 to 6ee1978 Compare June 11, 2026 17:29
@matthewhall2

Copy link
Copy Markdown
Contributor Author

thanks @nbhuiyan , I somehow missed these. I've now addressed the comments

@matthewhall2 matthewhall2 requested a review from nbhuiyan June 11, 2026 17:36
@mpirvu

mpirvu commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity xlinux,plinux,alinux,zlinux jdk21 depends eclipse-omr/omr#8209

@matthewhall2

Copy link
Copy Markdown
Contributor Author

aarch64 testList_3 fail is #17270
aarch64 failure testList_2 is unclear atm, but there are many issues open with the same "Agent communication error: java.net.SocketException: Broken pipe"

plinux sanity.functional and sanity.openjdk are #24139

@mpirvu

mpirvu commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

zlinux failures
jdk_collections_0

11:01:11  Assertion failed at /home/jenkins/workspace/Build_JDK21_s390x_linux_Personal/openj9/runtime/compiler/codegen/J9CodeGenerator.cpp:5270: false
11:01:11  VMState: 0x0005ff09
11:01:11  	orphaned const refs: obj3
11:01:11  	the const provenance graph is missing at least one edge

I have seen this when doing perf experiments with constrefs enabled on x86-64.

jdk_int128vector_j9_0, jdk_byte128vector_j9_0, jdk_long128vector_j9_0, jdk_short128vector_j9_0 fail without a clear error message. We have seen this type of failure before: #24071

11:12:57  #VECTOR API: Vectorized using vstoreiVector128Int32 in Int128VectorTests.withInt128VectorTests(Ljava/util/function/IntFunction;)V at scorching 
11:12:57  java.lang.Exception: config failures: 0, test failures: 1
11:12:57  	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:111)
11:12:57  	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:63)
11:12:57  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
11:12:57  	at java.base/java.lang.reflect.Method.invoke(Method.java:586)
11:12:57  	at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
11:12:57  	at java.base/java.lang.Thread.run(Thread.java:1600)

@mpirvu

mpirvu commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity win,aix,xmac,amac jdk21 depends eclipse-omr/omr#8209

@matthewhall2

matthewhall2 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@mpirvu

I have seen this when doing perf experiments with constrefs enabled on x86-64

have you seen this without my changes? I'm currently running internal tests with c40c05b cherry-picked to see if that fixes it, as I don't think it's the changes in this PR

@mpirvu

mpirvu commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

have you seen this without my changes? I'm currently running internal tests with c40c05b cherry-picked to see if that fixes it, as I don't think it's the changes in this PR

@nbhuiyan Testing from this PR hits the same assert I was hitting when running AcmeAir/Daytrader with your prototype. Did that prototype include this change?

@nbhuiyan

Copy link
Copy Markdown
Member

Testing from this PR hits the same assert I was hitting when running AcmeAir/Daytrader with your prototype. Did that prototype include this change?

The prototype I shared with you included the change in eclipse-omr/omr#8209, which then results in the assertion failure due to missing CPG edges. That needs to be addressed first so that the tests pass. I am working on that.

- Modifies getMethodHandleTableEntryIndex to use the asTypeCache when
  there is no exact MethodType match and the cache is non-null.
- folds the call to a const ref load when the KOI is not UNKNOWN

Signed-off-by: Matthew Hall <matthew.hall3@outlook.com>
@matthewhall2 matthewhall2 force-pushed the fold_vh_checkGenericType branch from 6ee1978 to f5605da Compare June 18, 2026 13:47
@matthewhall2

Copy link
Copy Markdown
Contributor Author

rebase was needed to fix the mac build errors

@mpirvu

mpirvu commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Jenkins test sanity all jdk21 depends eclipse-omr/omr#8209

@mpirvu

mpirvu commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

windows had one SCC test fail due to not being able to destroy the SCC. This is a known issue.
openjdk on zlinux had the usual vectorAPI failures: jdk_int128vector_j9_0, jdk_byte128vector_j9_0, jdk_long128vector_j9_0, jdk_short128vector_j9_0:

18:22:07  java.lang.Exception: config failures: 0, test failures: 1
18:22:07  	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:111)
18:22:07  	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:63)
18:22:07  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
18:22:07  	at java.base/java.lang.reflect.Method.invoke(Method.java:586)
18:22:07  	at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
18:22:07  	at java.base/java.lang.Thread.run(Thread.java:1600)

@mpirvu

mpirvu commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Since all test failures are accounted for and all review comments have been addresses, this PR is ready to be merged.

@mpirvu mpirvu merged commit 21a124f into eclipse-openj9:master Jun 19, 2026
25 of 28 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Done in Adopt OpenJDK MethodHandles Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

5 participants