On Tue, 2 Jun 2026 12:20:00 GMT, Roland Westrelin <[email protected]> wrote:

>> The bug is caused by a mismatch in the way `LoadFlat` and `StoreFlat`
>> are handled in `ConnectionGraph::find_inst_mem()` and
>> `MemNode::optimize_simple_memory_chain()`. Once EA has run a first
>> time, found some value array allocations as non escaping, and removed
>> their `LoadFlat`/`StoreFlat`, igvn runs and is able to further
>> transform the memory subgraph which introduces inconsistencies that
>> are then caught the next time EA runs.
>> 
>> The fix tries to make sure `ConnectionGraph::find_inst_mem()` and
>> `MemNode::optimize_simple_memory_chain()` handle `LoadFlat` and
>> `StoreFlat` similarly.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Roland Westrelin has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains six commits:
> 
>  - Merge branch 'lworld' into JDK-8385328
>  - more
>  - review
>  - whitespaces
>  - more
>  - test & fix

src/hotspot/share/opto/escape.cpp line 4566:

> 4564:     }
> 4565:   }
> 4566:   assert(t_store_base->instance_id() != toop->instance_id(), "should 
> not alias");

I'm not an EA expert, but I'm still not convinced that we can compare 
instance_id() her directly here without checking for is_known_instance() or 
unknown edge pointing to `phantom_obj`.  Also, I think such a test would be 
better as TypeOopPtr helper method.  (It is a pet peeve of mine that we expose 
the implementation detail of how instance IDs and alias IDs are integers.)
Maybe an EA expert like @vnkozlov could check this too.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2489#discussion_r3345539854

Reply via email to