RFR: 8347997: assert(false) failed: EA: missing memory path [v2]

[Tobias Hartmann]

On Fri, 24 Jan 2025 18:02:15 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> C2's Escape Analysis does not recognize pattern where one input of memory `Phi` node is `MergeMem` node and an other is RAW store. This pattern is created by Continuation pinning intrinsic. As result EA complains about strange memory graph.
>> 
>> I suggest to add second `MergeMem` between Store and Phi nodes by calling `reset_memory()`. EA recognize such patter and removes allocations.
>> 
>> I checked generated assembler pinning code and it is the same as before. The only difference in the test is eliminated allocations.
>> 
>> I moved Uncommon code up to avoid resetting memory - it is already done at the beginning of this intrinsic code.
>> 
>> We should not use `uncommon_trap_exact()` for `Deoptimization::Action_none` - It is used for other actions to prevent changing them to `Action_none`.
>> 
>> Tested tier1-5, hs-xcomp, hs-comp-stress
>> Added new regression test based on reproducer from bug report.
>
> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update test

Looks good to me otherwise.

> The only difference in the test is eliminated allocations.

Sounds like a good opportunity for an IR framework test but could be done as follow-up (starter) RFE.

test/hotspot/jtreg/compiler/intrinsics/TestContinuationPinningAndEA.java line 27:

> 25:  * @test
> 26:  * @bug 8347997
> 27:  * @summary Test that Cintinuation.pin() and unpin() intrinsics work with EA.

Suggestion:

 * @summary Test that Continuation.pin() and unpin() intrinsics work with EA.

test/hotspot/jtreg/compiler/intrinsics/TestContinuationPinningAndEA.java line 36:

> 34: public class TestContinuationPinningAndEA {
> 35: 
> 36:   static class FailsEA {

Should use 4-whitespace indentation since it's Java code.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23284#pullrequestreview-2577227944
PR Review Comment: https://git.openjdk.org/jdk/pull/23284#discussion_r1931596532
PR Review Comment: https://git.openjdk.org/jdk/pull/23284#discussion_r1931597080