On Fri, 30 Jan 2026 15:31:56 GMT, Frederic Parain <[email protected]> wrote:
>> Quan Anh Mai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add assert
>
> src/hotspot/share/ci/ciField.hpp line 189:
>
>> 187: // accessed in a non-atomic manner. This method must not depend on
>> the fact that the field cannot
>> 188: // be accessed racily (e.g. it is a strict final field), as if the
>> holder object is flattened
>> 189: // as a field that is not strict final, this property is lost.
>
> Thank you for this comment which makes this tricky point explicit.
> Question about C2 though, let's consider an identity object with a strict
> final field that is flattened using the NULLABLE_NON_ATOMIC_FLAT layout, will
> C2 consider the fields inside the flat field as constants?
I have added a more detailed explanation and the rationality behind the
decision. For `NULLABLE_NON_ATOMIC_FLAT`, C2 will consider the whole field as
constant, including its subfields.
> test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/ValueCompositionTest.java
> line 148:
>
>> 146: }
>> 147: FieldLayoutAnalyzer.FieldBlock f1 = cl.getFieldFromName("val1",
>> false);
>> 148:
>> Asserts.assertEquals(FieldLayoutAnalyzer.LayoutKind.NULLABLE_NON_ATOMIC_FLAT,
>> f1.layoutKind());
>
> I understand that this change is needed to make the test pass with now
> UseNullableNonAtomicValueFlattening being enabled by default, but the whole
> test should be in fact rewritten to support cases where nullable atomic and
> nullable non atomic layouts can be enabled or disabled. I've filed
> JDK-8376814 to track this issue.
> Instead of rewriting the assert, could you just comment the problematic lines
> and put a comment referring to JDK-8376814? The same request applies to the
> other changes in this file.
Got it, I simply disabled the flag in this test.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1970#discussion_r2747059722
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1970#discussion_r2747051002
