[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