[lworld] RFR: 8351006: [lworld] Refactor LoadNode::Identity of an InlineType into can_see_stored_value [v2]

[Christian Hagedorn]

On Sun, 2 Mar 2025 18:27:18 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> This patch moves the logic of `LoadNode::Identity` that tries to look through an `InlineTypeNode` to `can_see_stored_value`. This also fixes 2 issues:
>> 
>> - `Identity` should not return a new node
>> - `LoadUS` from a `short` is not a mismatched access and hence can return a wrong value
>> 
>> Please take a look and leave your reviews, thanks a lot.
>
> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   assert non-larval

> > It's now local to memnode.cpp which is huge.
> 
> If it is a method in `MemNode`, it needs to be `protected` since it is used by `LoadNNode`

That's true, it would need to be `protected` but I guess that's okay.

> so it is still effectively visible to the whole `memnode.cpp`, though.

Not entirely, there are other methods of classes in `memnode.cpp` that do not inherit from `MemNode` like `MergeMemNode` or `MemBarNode`. That's why I thought it would be less visible/clearer when moving it to the `MemNode` class instead where the method is called from (or from sub classes). But I don't have a strong opinion and leave it up to you to make the final decision.

I'm running some testing, will report the results back tomorrow.

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

Marked as reviewed by chagedorn (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1387#pullrequestreview-2665343667
PR Comment: https://git.openjdk.org/valhalla/pull/1387#issuecomment-2704672157