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

Quan Anh Mai qamai at openjdk.org
Mon May 26 15:30:04 UTC 2025


On Mon, 26 May 2025 15:20:21 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Merge branch 'lworld' into loadstoreflat
>  - cast_to_flat_array
>  - weird layout
>  - typo
>  - generalize weird layout
>  - fix array of nullable empty type
>  - fix array field calculations
>  - array
>  - convert_from_payload
>  - add tests
>  - ... and 1 more: https://git.openjdk.org/valhalla/compare/255dc257...e49bbc8c

Thanks for your reviews, I have addressed them. I notice there might be some additional issues here, too:

- `AddPNode::Value` does not work properly with pointers into arrays.
- The type system around arrays seems unsound. We don't have a way to express `maybe_atomic`.

I will investigate these further.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1470#issuecomment-2910103305


More information about the valhalla-dev mailing list