[lworld] RFR: 8357474: [lworld] Consolidate load/store flat inline types [v8]

[Christian Hagedorn]

On Fri, 23 May 2025 06:33:48 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> This patch consolidates loading and storing of flat inline types. This makes it so that `make_from_flat` and `store_flat` will take a pointer to the target location. `make_from_flat_array` and `store_flat_array` are split out into their own methods which receive an `idx` parameter. This also fixes some issues I theorycrafted:
>> 
>> - Improper loading/storing of atomic elements in a non-atomic container. This is not supported at the moment but it would be great to prepare for it.
>> - `is_naturally_atomic` checks `nof_declared_nonstatic_fields`, which is incorrect if the sole field has multiple fields. I refactored it into `ciInlineKlass`, the decision to do atomic loads/stores is also delegated to the callee, the caller only needs to know whether the operations act as if they are atomic, not whether they are actually executed atomically.
>> - `null_free` of an oop can only be trusted if the container itself is null-free. For example this case:
>> 
>>         value class StringHolder {
>>             @NullRestricted
>>             String s;
>>         }
>> 
>>         value class StringHolderHolder {
>>             // flatten
>>             StringHolder v;
>>         }
>> 
>> Then `v.s` cannot be trusted to be non-null because `v` can be null.
>> 
>> This also allows straightforward implementation of https://bugs.openjdk.org/browse/JDK-8349110.
>> 
>> Please let me know what you think. Thanks very much.
>
> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   weird layout

src/hotspot/share/opto/parse2.cpp line 129:

> 127:           ciArrayKlass* array_klass = ciArrayKlass::make(vk, /* flat */ true, is_null_free, maybe_atomic);
> 128:           const TypeAryPtr* arytype = TypeOopPtr::make_from_klass(array_klass)->isa_aryptr();
> 129:           arytype = arytype->cast_to_exactness(true);

Nice refactoring! Just noticed this here: IIUC, we don't know whether it's null-free or not here but we still set it to exact (was like that before already) which we should probably not? But it looks like the way we later use the `CastPP` in `make_from_flat_array()`, it does not cause problems because we will feed it into a nullable and null-free cast which is correctly set as exact.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1470#discussion_r2107422042