[lworld] RFR: 8376135: [lworld] Add JIT support for NULLABLE_NON_ATOMIC_FLAT layout

Tobias Hartmann thartmann at openjdk.org
Tue Jan 27 07:02:32 UTC 2026


On Mon, 26 Jan 2026 14:01:08 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> This PR implements the support of `NULLABLE_NON_ATOMIC_FLAT` layout in the JITs. There is nothing to do in C2. In C1, I need to implement loading/storing nullable value of a non-atomic field.
> 
> The test `TestValueClasses` is failing with `-XX:-TieredCompilation`, which I suspect is due to the substitutability test handling padding bytes incorrectly.
> 
> Please kindly review what there are for now, thanks a lot.

Thanks for jumping right on this, Quan Anh! The fix looks good to me, I just added a few minor comments.

> The test TestValueClasses is failing with -XX:-TieredCompilation, which I suspect is due to the substitutability test handling padding bytes incorrectly.

I thought this got fixed - do we have a tracking bug for this?

src/hotspot/share/c1/c1_GraphBuilder.cpp line 2067:

> 2065:                   Value nm_offset = append(new Constant(new LongConstant(offset + inline_klass->null_marker_offset_in_payload())));
> 2066:                   Value nm = append(new UnsafeGet(T_BOOLEAN, obj, nm_offset, false));
> 2067:                   result = append(new IfOp(nm, Instruction::neq, int_zero, new_instance, object_null, state_before, false));

Would be nice to replace this code in the LIRGenerator by HIR instructions as well:
https://github.com/openjdk/valhalla/blob/5c3e588a00ba28b2f9d095f5cbc86fa63958c21a/src/hotspot/share/c1/c1_LIRGenerator.cpp#L2124-L2127

But IIRC, that was non-trivial and is definitely out-of-scope of this PR.

src/hotspot/share/ci/ciInlineKlass.cpp line 159:

> 157: }
> 158: 
> 159: // All fields of this object is zero even if they can be null-free. As a result, this object should

Suggestion:

// All fields of this object are zero even if they are null-free. As a result, this object should

src/hotspot/share/opto/parse3.cpp line 285:

> 283:     inc_sp(1);
> 284:     bool is_immutable = field->is_final() && field->is_strict();
> 285:     bool atomic = vk->must_be_atomic() || !field->is_null_free();

Can `must_be_atomic` be removed now?

src/hotspot/share/runtime/globals.hpp line 842:

> 840:                                                                             \
> 841:   product(bool, UseNullableNonAtomicValueFlattening, true,                  \
> 842:            "Allow the JVM to flatten some strict final non-static fields")  \

Suggestion:

          "Allow the JVM to flatten some strict final non-static fields")  \

-------------

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1970#pullrequestreview-3709286745
PR Review Comment: https://git.openjdk.org/valhalla/pull/1970#discussion_r2730520300
PR Review Comment: https://git.openjdk.org/valhalla/pull/1970#discussion_r2730528922
PR Review Comment: https://git.openjdk.org/valhalla/pull/1970#discussion_r2730533413
PR Review Comment: https://git.openjdk.org/valhalla/pull/1970#discussion_r2730415298


More information about the valhalla-dev mailing list