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

[Quan Anh Mai]

On Mon, 26 May 2025 09:07:24 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   weird layout
>
> src/hotspot/share/opto/graphKit.cpp line 1880:
> 
>> 1878:   arytype = arytype->cast_to_exactness(true);
>> 1879:   arytype = arytype->cast_to_not_null_free(is_not_null_free);
>> 1880:   array = _gvn.transform(new CheckCastPPNode(control(), array, arytype, ConstraintCastNode::StrongDependency));
> 
> Did you hit an issue without this?

In principle, when we cast the array to null-free atomic, the result type depends on 2 dominating tests, which are the null-free test and the atomic test. As a result, we cannot use `RegularDependency` here, because `RegularDependency` means that the cast depends only on the test which is its control input.

> src/hotspot/share/opto/inlinetypenode.cpp line 724:
> 
>> 722:     region->init_req(2, kit->top());
>> 723:   } else {
>> 724:     region = new RegionNode(4);
> 
> For simplicity, can't we always have a `RegionNode` with 4 inputs here and simply leave `in(3)` as top/nullptr if we don't need it?

Oh that's right, I forgot about that

> src/hotspot/share/opto/inlinetypenode.cpp line 1387:
> 
>> 1385:         assert(vk->has_non_atomic_layout(), "");
>> 1386:         // TODO 8350865 Is the conversion to/from payload folded? We should wire this directly.
>> 1387:         // Also remove the PreserveReexecuteState in Parse::array_load when buffering is no longer possible.
> 
> Now that you removed the `convert_to_payload` hack, this comment should be removed, right? Also, buffering should no longer be possible and the referenced code in `Parse::array_load` can be removed.

Yes you are right, I have done that.

> src/hotspot/share/opto/parse2.cpp line 131:
> 
>> 129:           arytype = arytype->cast_to_exactness(true);
>> 130:           arytype = arytype->cast_to_not_null_free(is_not_null_free);
>> 131:           Node* flat_array = gvn().transform(new CastPPNode(control(), array, arytype));
> 
> I assume you are not using `flat_array_element_address` anymore because you don't need the address but only want to cast the array but I think this casting should then be factored out. Also, why don't you need a `ConstraintCastNode::StrongDependency` here?

That's a good idea, I refactored it into `GraphKit::cast_to_flat_array` and the element address can be trivially obtained from `array_element_address`.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1470#discussion_r2107556905
PR Review Comment: https://git.openjdk.org/valhalla/pull/1470#discussion_r2107557400
PR Review Comment: https://git.openjdk.org/valhalla/pull/1470#discussion_r2107559526
PR Review Comment: https://git.openjdk.org/valhalla/pull/1470#discussion_r2107558685