Gate IDENTITY modifier on Valhalla preview features in native code by konark2010 · Pull Request #23286 · eclipse-openj9/openj9

konark2010 · 2026-01-29T20:51:53Z

Description:

Previously, the IDENTITY modifier was enforced in the Java implementation. This change moves the gate to the native implementation, ensuring that the JVM reports the IDENTITY class modifier only when Valhalla preview features are enabled at runtime.

What changed:

Native code now checks the Valhalla preview runtime flag before setting the IDENTITY modifier.
Fixed IsIdentityTest failures, including array classes.

Motivation:

To ensure JVM behavior is consistent with Valhalla design expectations.

Impact:

JVM correctness for identity/value type detection.
Test suite passes reliably.

konark2010 · 2026-01-29T20:52:02Z

@hangshao0 @theresa-m can you please review this

jcl/src/java.base/share/classes/java/lang/Class.java

keithc-ca

Changes are also required in runtime/vm/FastJNI_java_lang_Class.cpp and runtime/vm/romclasses.c.

jcl/src/java.base/share/classes/java/lang/Class.java

runtime/compiler/env/VMJ9.cpp

runtime/vm/BytecodeInterpreter.hpp

jcl/src/java.base/share/classes/java/lang/Class.java

runtime/compiler/env/VMJ9.cpp

keithc-ca · 2026-02-02T19:28:09Z

Please address all feedback, including #23286 (review), before requesting the next review.

jcl/src/java.base/share/classes/java/lang/Class.java

runtime/compiler/env/VMJ9.cpp

runtime/vm/FastJNI_java_lang_Class.cpp

runtime/compiler/env/VMJ9.cpp

runtime/vm/romclasses.c

keithc-ca · 2026-02-02T22:49:57Z

Please update the commit message: This is more about correcting the behavior of the JVM than fixing a test. Yes, it's important that we address test failures, and it's reasonable to elaborate on that in the body of the commit message, but the summary line should be about what the change will do the the JVM. The title of the pull request and the description should be consistent with that updated commit message.

keithc-ca · 2026-02-03T13:09:19Z

The title of the pull request and the description should be consistent with that updated commit message

Please also do that.

runtime/compiler/env/VMJ9.cpp

This change fixes JVM behavior related to the IDENTITY modifier. The JVM should report the IDENTITY modifier only when Valhalla preview features are enabled at runtime. The native implementation is updated to check the preview runtime flag before reporting the modifier. This change also fixes IsIdentityTest failures, including array cases. Signed-off-by: Konark Shah <konarkshah2010@gmail.com>

keithc-ca · 2026-02-03T18:54:17Z

@dsouzai Can you review or recommend a reviewer for the JIT changes here?

hangshao0 · 2026-02-03T19:23:12Z

Added @hzongaro to review the JIT change.
FYI @a7ehuo

hangshao0 · 2026-02-03T19:30:02Z

In that case please double check that the valhalla functional and OpenJDK tests are all passing @konark2010.

Once double checked, please confirm if they are all passing.

konark2010 · 2026-02-03T19:33:34Z

In that case please double check that the valhalla functional and OpenJDK tests are all passing @konark2010.

Once double checked, please confirm if they are all passing.

yes am checking the functional tests and the OpenJDK test will update here once done

dsouzai · 2026-02-03T21:01:24Z

@dsouzai Can you review or recommend a reviewer for the JIT changes here?

Yeah I think Annabelle and/or Henry are appropriate reviewers for this.

hzongaro

I think the JIT compiler changes look correct. Thanks!

keithc-ca · 2026-02-04T16:06:18Z

Jenkins test sanity.functional,extended xlinuxval jdknext

konark2010 · 2026-02-04T16:19:39Z

There is one of the functional test failing when I checked on my local, am fixing it, will update here once done

keithc-ca · 2026-02-04T18:20:31Z

one functional test failing

Which test is that? None failed in the build I launched.

konark2010 · 2026-02-04T18:21:54Z

one functional test failing

Which test is that? None failed in the build I launched.

Yes, actually the one which failed was due to my Mac so that can be ignored, so functional tests are good

konark2010 · 2026-02-04T18:25:09Z

Also I confirmed the OPEN-JDK tests are passing here is the link to it https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/58045/ , so now the PR should be good to move ahead

keithc-ca · 2026-02-04T18:25:59Z

@hangshao0 @theresa-m Could you please complete your reviews?

theresa-m · 2026-02-04T21:42:55Z

jcl/src/java.base/share/classes/java/lang/Class.java

 	} else {
 		rawModifiers &= Modifier.PUBLIC | Modifier.PRIVATE | Modifier.PROTECTED
 /*[IF INLINE-TYPES]*/
-				| AccessFlag.IDENTITY.mask() | Modifier.STRICT


Did you intend to remove this?

keith asked me to remove this #23286 (comment)

Yes okay. I believe the other uses of Modifier.Strict should also be removed to clean up old code as well @keithc-ca ? That can be a seperete change.

I pointed out that strict does not apply to types, only methods; see #23286 (comment).

sounds good.

hangshao0 · 2026-02-05T14:57:13Z

Class.isValue() may need to always return false if --enable-preview is off at runtime. This can also be done in the native code.

@konark2010 Could you look at this and open a PR if necessary.

keithc-ca requested review from hangshao0 and theresa-m January 29, 2026 20:55

keithc-ca reviewed Jan 29, 2026

View reviewed changes

jcl/src/java.base/share/classes/java/lang/Class.java Outdated Show resolved Hide resolved

konark2010 force-pushed the fix-for-failing-identity-test branch from a57421e to 00d2a6e Compare January 29, 2026 21:34

konark2010 requested a review from keithc-ca January 29, 2026 21:34

theresa-m reviewed Jan 29, 2026

View reviewed changes

jcl/src/java.base/share/classes/java/lang/Class.java Outdated Show resolved Hide resolved

hangshao0 added comp:vm project:valhalla Used to track Project Valhalla related work labels Jan 30, 2026

konark2010 requested a review from dsouzai as a code owner February 2, 2026 18:00

keithc-ca requested changes Feb 2, 2026

View reviewed changes

konark2010 requested review from keithc-ca and theresa-m February 2, 2026 18:54

keithc-ca reviewed Feb 2, 2026

View reviewed changes

jcl/src/java.base/share/classes/java/lang/Class.java Show resolved Hide resolved

jcl/src/java.base/share/classes/java/lang/Class.java Outdated Show resolved Hide resolved

runtime/compiler/env/VMJ9.cpp Show resolved Hide resolved

konark2010 requested a review from keithc-ca February 2, 2026 21:36

keithc-ca requested changes Feb 2, 2026

View reviewed changes

konark2010 requested a review from keithc-ca February 2, 2026 22:26

keithc-ca reviewed Feb 2, 2026

View reviewed changes

runtime/vm/romclasses.c Outdated Show resolved Hide resolved

konark2010 requested a review from keithc-ca February 2, 2026 22:39

keithc-ca reviewed Feb 2, 2026

View reviewed changes

runtime/vm/romclasses.c Outdated Show resolved Hide resolved

konark2010 requested a review from keithc-ca February 2, 2026 22:44

konark2010 force-pushed the fix-for-failing-identity-test branch from 44721ba to 8d1b758 Compare February 2, 2026 22:49

konark2010 force-pushed the fix-for-failing-identity-test branch from 8d1b758 to e5182bb Compare February 2, 2026 23:01

theresa-m suggested changes Feb 3, 2026

View reviewed changes

runtime/compiler/env/VMJ9.cpp Show resolved Hide resolved

konark2010 force-pushed the fix-for-failing-identity-test branch from e5182bb to c69772a Compare February 3, 2026 15:43

konark2010 changed the title ~~Fix IsIdentityClassTest for array IDENTITY modifiers~~ Gate IDENTITY modifier on Valhalla preview features in native code Feb 3, 2026

hangshao0 requested review from hzongaro and removed request for dsouzai February 3, 2026 19:21

hangshao0 added the comp:jit label Feb 3, 2026

hangshao0 added this to Valhalla L-World Feb 3, 2026

github-project-automation bot moved this from TODO: VM to Needs review in Valhalla L-World Feb 3, 2026

github-project-automation bot moved this to TODO: VM in Valhalla L-World Feb 3, 2026

hzongaro approved these changes Feb 4, 2026

View reviewed changes

keithc-ca approved these changes Feb 4, 2026

View reviewed changes

github-project-automation bot moved this from Needs review to Reviewer approved in Valhalla L-World Feb 4, 2026

hangshao0 approved these changes Feb 4, 2026

View reviewed changes

konark2010 requested a review from theresa-m February 4, 2026 20:55

theresa-m reviewed Feb 4, 2026

View reviewed changes

theresa-m approved these changes Feb 4, 2026

View reviewed changes

keithc-ca merged commit c6871e9 into eclipse-openj9:master Feb 4, 2026
6 checks passed

github-project-automation bot moved this from Reviewer approved to Done in Valhalla L-World Feb 4, 2026

This was referenced Feb 5, 2026

Update ProblemList_openjdkvalhalla-openj9.txt adoptium/aqa-tests#6860

Closed

Update ProblemList_openjdkvalhalla-openj9.txt adoptium/aqa-tests#6876

Closed

Update ProblemList_openjdkvalhalla-openj9.txt adoptium/aqa-tests#6878

Merged

Conversation

konark2010 commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

konark2010 commented Jan 29, 2026

Uh oh!

Uh oh!

Uh oh!

keithc-ca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keithc-ca commented Feb 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keithc-ca commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keithc-ca commented Feb 3, 2026

Uh oh!

Uh oh!

keithc-ca commented Feb 3, 2026

Uh oh!

hangshao0 commented Feb 3, 2026

Uh oh!

hangshao0 commented Feb 3, 2026

Uh oh!

konark2010 commented Feb 3, 2026

Uh oh!

dsouzai commented Feb 3, 2026

Uh oh!

hzongaro left a comment

Choose a reason for hiding this comment

Uh oh!

keithc-ca commented Feb 4, 2026

Uh oh!

konark2010 commented Feb 4, 2026

Uh oh!

keithc-ca commented Feb 4, 2026

Uh oh!

konark2010 commented Feb 4, 2026

Uh oh!

konark2010 commented Feb 4, 2026

Uh oh!

keithc-ca commented Feb 4, 2026

Uh oh!

theresa-m Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

konark2010 Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

theresa-m Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

keithc-ca Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

theresa-m Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hangshao0 commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

konark2010 commented Jan 29, 2026 •

edited

Loading

keithc-ca commented Feb 2, 2026 •

edited

Loading