Compact layout updates for jdk.internal.misc.Unsafe by theresa-m · Pull Request #23301 · eclipse-openj9/openj9

theresa-m · 2026-02-03T15:08:15Z

Add a new compiler flag COMPACT_LAYOUT for Java changes. This flag is disabled by default.
Update compareAndExchangeByte/Boolean/Char/Short
Update getByte/Boolean/Char/Short
Update putByte/Boolean/Char/Short

Related: #21251

I have confirmed locally that the Valhalla functional tests that do not rely on the JIT are passing. The tests in functional/UnsafeTest pass as well using -Xint.

amicic · 2026-02-24T16:20:38Z

+	 * @return the value stored in the object field before the update
+	 */
+	VMINLINE U_8
+	inlineMixedObjectCompareAndExchangeU8(J9VMThread *vmThread, j9object_t destObject, UDATA destOffset, U_8 compareValue, U_8 swapValue, bool isVolatile = false)


the rest of barrier code seems to be under the build flag, but not here

There are existing J9VM_OPT_VALHALLA_COMPACT_LAYOUTS flags for these methods at line 620 and 952.

theresa-m · 2026-03-17T16:42:32Z


 TR::Node *TR_J9InlinerPolicy::createUnsafeAddressWithOffset(TR::Node *unsafeCall)
 {
+#if defined(J9VM_OPT_VALHALLA_COMPACT_LAYOUTS)


@hzongaro fyi this method is causing a compilation error since the new J9_SUN_FIELD_OFFSET_MASK are 64 bits.

Does this result in a build-time compilation error (from the C++ compiler) or a compilation failure/crash for the JIT compiler?

It results in a build time error.

openj9/runtime/compiler/optimizer/InlinerTempForJ9.cpp:552:42: error: implicit conversion from 'unsigned long' to 'int32_t' (aka 'int') changes value from 4611686018427387903 to -1 [-Werror,-Wconstant-conversion] TR::Node::iconst(unsafeCall, ~(J9_SUN_FIELD_OFFSET_MASK)))); ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~

I guess with the original value for J9_SUN_FIELD_OFFSET_MASK, it would work in both 32-bit and 64-bit modes. The JIT doesn't usually conditionally compile out sections based on whether the target is 32-bit or 64-bit, so I don't want to suggest using conditional compilation here. May I suggest trying to cast the J9_SUN_FIELD_OFFSET_MASK to an int32_t in the call to TR::Node::iconst?

#else /* defined(J9VM_OPT_VALHALLA_COMPACT_LAYOUTS) */ if (comp()->target().is64Bit()) { TR::Node *constNode = TR::Node::lconst(unsafeCall, ~(J9_SUN_FIELD_OFFSET_MASK)); return TR::Node::create(TR::aladd, 2, unsafeCall->getChild(1), TR::Node::create(TR::land, 2, unsafeCall->getChild(2), constNode)); } return TR::Node::create(TR::aiadd, 2, unsafeCall->getChild(1), TR::Node::create(TR::iand, 2, TR::Node::create(TR::l2i, 1, unsafeCall->getChild(2)), TR::Node::iconst(unsafeCall, ~((int32_t) J9_SUN_FIELD_OFFSET_MASK)))); #endif /* defined(J9VM_OPT_VALHALLA_COMPACT_LAYOUTS) */

We may be moving the tags to the high bits (of a long offset), so we should not assume that the value is 32 bit.

We may be moving the tags to the high bits (of a long offset), so we should not assume that the value is 32 bit.

Ah, sorry - I was thinking the tag values would still be less than 2^32 in 32-bit mode. However, this is masking off the tag values for use in an address calculation, so I think it would be safe to consider only the low-order 32 bits of the mask value.

Now that I look at it more closely, I see the case under defined(J9VM_OPT_VALHALLA_COMPACT_LAYOUTS) is the same as the 64-bit mode non-compact layout version, so I think it would be safe to remove that entire #if defined block and leave the method as it originally was, except casting the value in the call to TR::Node:iconst to an int32_t, and then perhaps add a comment pointing out that even if the tag bits are in the high-order 32 bits, only the low order 32 bits participate in the address calculation for 32 bit targets, so it’s safe to cast the complemented mask value to a uint32_t.

Perhaps too it would be better to use (int32_t) (~(J9_SUN_FIELD_OFFSET_MASK)) rather than ~((int32_t) J9_SUN_FIELD_OFFSET_MASK) as I originally suggested.

Agreed, the constant may be explicitly declared as a U_64 if we move the bits. The current assumption that the bits are encoded in the offset will no longer be correct on 32-bit (only the low 32 bits are the offset, and tha tag bits are in the high 32 bits).

Thanks, I've added your suggestion

theresa-m · 2026-03-26T15:02:04Z

This compiles on win32 now http://vmfarm.rtp.raleigh.ibm.com/build_info.php?build_id=114779

hzongaro

JIT compiler-related changes look good to me. Thanks!

gacholio · 2026-03-27T22:07:14Z

I will look at this tomorrow.

hangshao0 · 2026-04-01T15:47:47Z

@amicic This is waiting for your approval.

- Add a new compiler flag COMPACT_LAYOUT for Java changes. This flag is disabled by default. - Update Unsafe compareAndExchangeByte/Boolean/Char/Short - Update Unsafe getByte/Boolean/Char/Short - Update Unsafe putByte/Boolean/Char/Short - Move static offset flags from least significant bits to most significant bits of U_64 and adjust variable types from UDATA to U_64 Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

hangshao0 · 2026-04-08T14:10:50Z

Jenkins test sanity.functional,extended alinuxval jdknext

hangshao0 · 2026-04-08T14:12:51Z

jenkins compile win,win32 jdk8

hangshao0 · 2026-04-08T14:13:54Z

jenkins test sanity.functional,extended zlinux jdk21

keithc-ca · 2026-04-08T19:17:34Z


+#if defined(J9VM_OPT_VALHALLA_COMPACT_LAYOUTS)
+/**
+ * Performs an atomic compare-and-exchange on a byte field of a mixed object


Missing period; also line 2183.

keithc-ca · 2026-04-08T19:18:34Z

+	{
+#if defined(J9VM_GC_ALWAYS_CALL_OBJECT_ACCESS_BARRIER)
+		return vmThread->javaVM->memoryManagerFunctions->j9gc_objaccess_mixedObjectCompareAndExchangeByte(vmThread, destObject, destOffset, compareValue, swapValue);
+#elif defined(J9VM_GC_COMBINATION_SPEC)


Missing comment.

keithc-ca · 2026-04-08T19:19:18Z

+		return result;
+#else /* J9VM_GC_ALWAYS_CALL_OBJECT_ACCESS_BARRIER */
+#error unsupported barrier
+#endif /* J9VM_GC_ALWAYS_CALL_OBJECT_ACCESS_BARRIER */


Mismatched comments (they should repeat the if condition, i.e. defined(J9VM_GC_ALWAYS_CALL_OBJECT_ACCESS_BARRIER) || defined(J9VM_GC_COMBINATION_SPEC)).

keithc-ca · 2026-04-08T19:20:40Z

+			offset = fieldID->offset;
+			offset |= J9_SUN_STATIC_FIELD_OFFSET_TAG;


There are several similar changes: Why?

I thought it would be easier to debug if something went wrong with changing the value of these flags.

A compiler may (hopefully will) elide the first assignment, effectively yielding the same code as for the old code, so debugging should be a similar experience either way.

keithc-ca · 2026-04-08T19:23:38Z

+				/* Aligned array access */
+				UDATA index = convertOffsetToIndex(currentThread, offset, logElementSize);
+				result = objectAccessBarrier->inlineIndexableObjectCompareAndExchangeU8(currentThread, object, index, compareValue, swapValue, true);
+			} else if (offset & J9_SUN_STATIC_FIELD_OFFSET_TAG) {


This should use J9_ARE_ANY_BITS_SET().

keithc-ca · 2026-04-08T19:25:57Z

 	{
 		U_8 value = (U_8)*(I_32*)_sp;
-		UDATA offset = (UDATA)*(I_64*)(_sp + 1);
+		U_64 offset = *(I_64*)(_sp + 1);


Mismatched types (several places):

U_64 offset = *(U_64 *)(_sp + 1);

Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

Address additional review comments from #23301

This reverts commit e3b1a77.

Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

theresa-m added comp:vm project:valhalla Used to track Project Valhalla related work labels Feb 3, 2026

theresa-m force-pushed the compact_layout_unsafe branch 4 times, most recently from 32cfb6e to 80edf79 Compare February 3, 2026 20:46

theresa-m force-pushed the compact_layout_unsafe branch from 80edf79 to ed01eb7 Compare February 18, 2026 16:29

theresa-m mentioned this pull request Feb 18, 2026

Add CAS API for 8-bit and 16 bit values eclipse-omr/omr#8133

Merged

theresa-m marked this pull request as ready for review February 18, 2026 18:32

theresa-m requested review from amicic, gacholio and hangshao0 February 18, 2026 18:33

hangshao0 reviewed Feb 18, 2026

View reviewed changes

Comment thread jcl/src/java.base/share/classes/jdk/internal/misc/Unsafe.java

Comment thread jcl/src/java.base/share/classes/jdk/internal/misc/Unsafe.java Outdated

theresa-m force-pushed the compact_layout_unsafe branch from ed01eb7 to ccbab69 Compare February 19, 2026 20:55

hangshao0 reviewed Feb 19, 2026

View reviewed changes

Comment thread jcl/src/java.base/share/classes/jdk/internal/misc/Unsafe.java

hangshao0 reviewed Feb 24, 2026

View reviewed changes

Comment thread runtime/oti/UnsafeAPI.hpp

amicic reviewed Feb 24, 2026

View reviewed changes

theresa-m marked this pull request as draft March 17, 2026 14:43

theresa-m force-pushed the compact_layout_unsafe branch 2 times, most recently from 777c68a to 8fcf4bc Compare March 17, 2026 16:40

theresa-m marked this pull request as ready for review March 17, 2026 16:40

theresa-m requested a review from hangshao0 March 17, 2026 16:40

theresa-m commented Mar 17, 2026

View reviewed changes

hangshao0 reviewed Mar 17, 2026

View reviewed changes

Comment thread jcl/src/java.base/share/classes/jdk/internal/misc/Unsafe.java

Comment thread jcl/src/java.base/share/classes/jdk/internal/misc/Unsafe.java Outdated

Comment thread runtime/oti/j9consts.h

Comment thread runtime/oti/j9consts.h Outdated

theresa-m force-pushed the compact_layout_unsafe branch from 8fcf4bc to ec38209 Compare March 17, 2026 20:19

theresa-m requested a review from hangshao0 March 17, 2026 20:19

hangshao0 reviewed Mar 17, 2026

View reviewed changes

Comment thread jcl/src/java.base/share/classes/jdk/internal/misc/Unsafe.java Outdated

hangshao0 reviewed Mar 17, 2026

View reviewed changes

Comment thread runtime/oti/j9consts.h Outdated

theresa-m force-pushed the compact_layout_unsafe branch 2 times, most recently from 05b6a85 to 8cd7062 Compare March 24, 2026 16:41

theresa-m force-pushed the compact_layout_unsafe branch 3 times, most recently from 066f053 to 1e897ca Compare March 26, 2026 15:01

theresa-m marked this pull request as ready for review March 26, 2026 15:01

hzongaro approved these changes Mar 27, 2026

View reviewed changes

gacholio approved these changes Mar 30, 2026

View reviewed changes

amicic approved these changes Apr 1, 2026

View reviewed changes

theresa-m force-pushed the compact_layout_unsafe branch from 1e897ca to f7987c3 Compare April 6, 2026 14:04

hangshao0 merged commit c9bd78f into eclipse-openj9:master Apr 8, 2026
12 checks passed

keithc-ca reviewed Apr 8, 2026

View reviewed changes

theresa-m added a commit to theresa-m/openj9 that referenced this pull request Apr 9, 2026

Address additional review comments from eclipse-openj9#23301

a952e45

Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

theresa-m mentioned this pull request Apr 9, 2026

Address additional review comments from pr #23301 #23669

Merged

pshipton mentioned this pull request Apr 9, 2026

jsr292Test ASSERTION FAILED runtime\jcl\common\java_dyn_methodhandle.c:571: (((J9ROMFieldShape *)(*romField)) != NULL) #23673

Closed

theresa-m added a commit to theresa-m/openj9 that referenced this pull request Apr 9, 2026

Address additional review comments from eclipse-openj9#23301

b3a1299

Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

theresa-m added a commit to theresa-m/openj9 that referenced this pull request Apr 10, 2026

Address additional review comments from eclipse-openj9#23301

e3b1a77

Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

hangshao0 added this to Valhalla L-World Apr 13, 2026

github-project-automation Bot moved this to TODO: VM in Valhalla L-World Apr 13, 2026

github-project-automation Bot moved this from TODO: VM to Done in Valhalla L-World Apr 13, 2026

keithc-ca added a commit that referenced this pull request Apr 13, 2026

Merge pull request #23669 from theresa-m/fix_review_23301

601eacd

Address additional review comments from #23301

theresa-m added a commit to theresa-m/openj9 that referenced this pull request Apr 14, 2026

Revert "Address additional review comments from eclipse-openj9#23301"

8afdf95

This reverts commit e3b1a77.

theresa-m mentioned this pull request Apr 27, 2026

Compact layout for instance field smaller than 32-bit #21251

Open

theresa-m mentioned this pull request May 11, 2026

Compact layout type updates for JVMTI #23855

Closed

dev-koan pushed a commit to dev-koan/openj9 that referenced this pull request May 27, 2026

Address additional review comments from eclipse-openj9#23301

1c9a5ae

Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>

		offset = fieldID->offset;
		offset \|= J9_SUN_STATIC_FIELD_OFFSET_TAG;

Uh oh!

Conversation

theresa-m commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theresa-m commented Mar 26, 2026

Uh oh!

hzongaro left a comment

Choose a reason for hiding this comment

Uh oh!

gacholio commented Mar 27, 2026

Uh oh!

hangshao0 commented Apr 1, 2026

Uh oh!

hangshao0 commented Apr 8, 2026

Uh oh!

hangshao0 commented Apr 8, 2026

Uh oh!

hangshao0 commented Apr 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

theresa-m commented Feb 3, 2026 •

edited

Loading