RFR: JDK-8316756: C2 EA fails with "missing memory path" when encountering unsafe_arraycopy stub call

[Vladimir Kozlov]

On Wed, 10 Jan 2024 13:35:17 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

> Before https://github.com/openjdk/jdk/pull/5259 the graph of the following program looked like this during Escape Analysis:
> 
> 
> static int test() {
>     MyClass obj = new MyClass(); // Non-escaping to trigger Escape Analysis
>     UNSAFE.copyMemory(null, SRC_BASE, null, DST_BASE, 4);
>     obj.x = 42;
>     return obj.x;
> }
> 
> With MemBarCPUOrder:
> <img width="395" alt="working" src="https://github.com/openjdk/jdk/assets/71546117/63a82554-49ff-4373-8567-4457b094093f">
> 
> Setting `RC_NARROW_MEM` flag in `LibraryCallKit::inline_unsafe_copyMemory()` removes the `428 MergeMem` node. Without a `MergeMem` node after `432 StoreB` EA failes in Phase 2 of `ConnectionGraph::split_unique_types(...)` when trying to push the allocation's users on the appropriate worklist - `429 CallLeafNoFP` is not an expected user of `428 StoreB`. Therefore the assert `"EA: missing memory path"` is hit. 
> Without MemBarCPUOrdera and after setting `RC_NARROW_MEM`:
> <img width="370" alt="failing" src="https://github.com/openjdk/jdk/assets/71546117/5d98c38c-d404-4ac4-bd11-bc1e1b7b481f">
> 
> 
> ### Proposed Fix
> Dropping the `RC_NARROW_MEM` flag in `LibraryCallKit::inline_unsafe_copyMemory()` causes the introduction of a `MergeMem` between `StoreB` and `CallLeafNoFP`, so the corresponding code in EA doesn't encounter a `CallLeafNoFP` anymore:
> <img width="336" alt="fixed" src="https://github.com/openjdk/jdk/assets/71546117/c538c199-6676-49a4-aaac-a2b3b8f11694">
> 
> Testing: Tier1-4 passed

This should be reviewed by @iwanowww.

I have too many question about this.

Based on code and test `UNSAFE.copyMemory()` copies "native" memory which should not affect anything.  [#5259](https://github.com/openjdk/jdk/pull/5259) sets RC_NARROW_MEM exactly for that as I understand.

Flag setting (StoreB nodes) in `JavaThread::_doing_unsafe_access` is also not affected but it is volatile field and these stores should be staying where they are. They can't go up or down. And it should be accomplished by some kind of barriers between `StoreB` and `unsafe_Arraycopy` call.  But the call's memory edge should not point to `StoreB` - it is incorrect since it does not affect that field in this case. Call's memory should point to root memory in this case.

Operating on fields of new `MyClass` object could be moved around and object can be eliminated since it does not escape.

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

PR Review: https://git.openjdk.org/jdk/pull/17347#pullrequestreview-1816177520