-
Notifications
You must be signed in to change notification settings - Fork 722
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
Don't assume lockword of class is at index 0 #20642
Conversation
@@ -1369,7 +1369,7 @@ Java_java_lang_invoke_MethodHandleNatives_resolve( | |||
} | |||
} | |||
|
|||
if ((0 != target) && ((0 != vmindex) || J9_ARE_ANY_BITS_SET(flags, MN_IS_METHOD | MN_IS_CONSTRUCTOR))) { | |||
if ((NULL != new_clazz) && ((0 != vmindex) || J9_ARE_ANY_BITS_SET(flags, MN_IS_METHOD | MN_IS_CONSTRUCTOR))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we limit this change only to affect instance field in project Valhalla.
Adding @TobiAjila as reviewer. This change is removing the assumption that visible instance field starts at offset != 0. Value type and example class in #20386 (comment) could have its visible instance field start at offset 0. |
85b19e5
to
1a1be93
Compare
Jenkins test sanity,extended xlinuxval jdknext |
jenkins test sanity.functional win jdk8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add a comment in the code that field could start from offset 0 in Valhalla.
1a1be93
to
c9599cd
Compare
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES) | ||
/* In project Valhalla fields may start at offset 0. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be restructured so there's only one if
statement, not an extra {
that would be unmatched, and eliminate duplication of the common subexpression:
if (
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
/* In project Valhalla fields may start at offset 0. */
(NULL != new_clazz)
#else /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
(0 != target)
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
&& ((0 != vmindex) || J9_ARE_ANY_BITS_SET(flags, MN_IS_METHOD | MN_IS_CONSTRUCTOR))
) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the change to match this.
c9599cd
to
cd7c592
Compare
With valuetypes it's possible for the lockword not to be the first field. Signed-off-by: Theresa Mammarella <[email protected]>
cd7c592
to
c7c6a13
Compare
Any more comments on this change? |
@keithc-ca ^^ |
With valuetypes it's possible for the lockword not to be the first field.
Unblocks #20386