[lworld] RFR: 8372113: [lworld] Fix various issues with TypeAryPtr and TypeAryKlassPtr [v6]

[Quan Anh Mai]

On Thu, 11 Dec 2025 18:24:46 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> src/hotspot/share/prims/jvm.cpp line 493:
>> 
>>> 491:   } else {
>>> 492:     props = ArrayKlass::ArrayProperties::NULL_RESTRICTED;
>>> 493:   }
>> 
>> Suggestion:
>> 
>>   ArrayKlass::ArrayProperties props = ArrayKlass::ArrayProperties::NULL_RESTRICTED;
>>   if (vk->has_non_atomic_layout()) {
>>     props |= ArrayKlass::ArrayProperties::NON_ATOMIC;
>>   }
>> 
>> 
>> Maybe double-check, if my suggestion even compiles :slightly_smiling_face: 
>> 
>> Regarding:
>> 
>>> When creating a refined ObjArrayKlass, we fall back to ref array if the corresponding flat layout is not present, which means that if the element type is not a LooselyConsistentValue, ValueClass::newNullRestrictedNonAtomicArray will make a ref array. This seems counter-intuitive, it should have fallen back to a null-restricted atomic array instead.
>> 
>> I tend to agree but this is still debatable. @fparain Thoughts?
>
> First of all, any logic deciding the layout of an array should be in ObjArrayKlass::array_layout_selection().
> We had layout logics in multiple places in the past, and they were getting out of sync very quickly.
> So let's try to keep the decision logic in a single place. (Which means that the jvm.cpp code has to be modified anyway to just pass NULL_RESTRICTED | NON_ATOMIC to the array factory).
> The fall back to the atomic layout makes sense in this case where the array creator asked for non-atomicity but the value class author didn't opt in non-atomicity.
> So the code in ObjArrayKlass::Array_layout_selection() would become:
> 
> 
> if (is_null_restricted(properties)) {
>     if (is_non_atomic(properties)) {
>       // Null-restricted + non-atomic
>       if (vk->maybe_flat_in_array()) {
>         if (vk->has_non_atomic_layout()) {
>           return ArrayDescription(FlatArrayKlassKind, properties, LayoutKind::NON_ATOMIC_FLAT);
>         } else if (vk->has_atomic_layout()) {
>           return ArrayDescription(FlatArrayKlassKind, properties, LayoutKind::ATOMIC_FLAT);
>         }
>       }
>       return ArrayDescription(RefArrayKlassKind, properties, LayoutKind::REFERENCE);
>     } else {

@fparain Thanks for your review, This is not about choosing a layout, however, it is about choosing a refined class for the array that is created by `ValueClass::newNullRestrictedNonAtomicArray`. If we go with your suggestion, then the array created by this method will have a different refined type from an array created by `ValueClass::newNullRestrictedAtomicArray`, which seems dangerous because C2 may try to assume that 2 classes having the same properties should be equal. As a result, I went with this change which asserts at `ObjArrayKlass::allocate_klass_with_properties` that non-atomic is only requested when it is possible to do so, and changed the call sites that may have this property popping up. The common point of those 2 sites is `ObjArrayKlass::klass_with_properties`, so I think it is less clear whether we should normalize the properties argument there instead. What do you think?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1755#discussion_r2612709444