RFR: 8325030: PhaseMacroExpand::value_from_mem_phi assert with "unknown node on this path" [v2]

[Theo Weidmann]

> The following code triggers an assert:
> 
> 
> class Test {
>     static class A {
>         int a;
>     }
> 
>     public static void main(String[] strArr) {
>         int i2 = 0;
>         for (int i = 0; i < 50; ++i)
>             try {
>                 synchronized (new A()) {
>                     synchronized (Test.class) {
>                         for (int var19 = 0; var19 < Integer.valueOf(i2);) {
>                             Integer.valueOf(var19);
>                         }
>                     }
>                 }
>                 for (int var8 = 0; var8 < 10000; ++var8) ;
>             } catch (ArithmeticException a_e) {
>             }
>     }
> }
> 
> 
> 
> #  Internal Error (/Users/theo/jdk/open/src/hotspot/share/opto/macro.cpp:435), pid=37385, tid=27651
> #  assert(false) failed: unknown node on this path
> 
> 
> when run with `-XX:-ProfileExceptionHandlers`. With different flag configurations this can be reproduced back as far as JDK 11. (See issue in JBS.)
> 
> The issue is caused during OSR compilation in macro expansion (`PhaseMacroExpand::eliminate_macro_nodes`):
> 
> During `eliminate_macro_nodes`, the boxing call `Integer.valueOf(var19)` (which can be seen in the graph below as node 359) is eliminated:
> 
> <img width="545" alt="Screenshot 2025-01-14 at 14 56 27" src="https://github.com/user-attachments/assets/233d0972-a477-4d7a-8cdc-adee3e670d45" />
> 
> Note that on the catch path we have MemBarReleaseLock node (462) whose control and memory input become top after 359 CallStaticJava is eliminated. This elimination is correct per se.
> 
> `PhaseMacroExpand::eliminate_macro_nodes` will then continue with the next macro node in its list, the allocation `new A()`, which it tries to eliminate using scalar replacement. Note that IGVN does not run in-between this macro elimination attempts.
> 
> During scalar replacement, while finding the values for each field and walking along the memory edges, 462 MemBarReleaseLock, whose memory input is still top, is hit in `PhaseMacroExpand::value_from_mem_phi`. `PhaseMacroExpand::value_from_mem_phi` does not expect top and the assert triggers.
> 
> A naive solution for this problem would be to run IGVN in between each macro elimination attempt in order to ensure the entire catch path with 462 MemBarReleaseLock is removed. This does indeed solve the problem but the performance impact of running IGVN in between each macro elimination is unclear.
> 
> Instead, we observe that `PhaseMacroExpand::value_from_mem_phi` tries to convert a memory phi to a value phi node. It is fine for a PhiNo...

Theo Weidmann has updated the pull request incrementally with two additional commits since the last revision:

 - Add regression test
 - Add test

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/23104/files
  - new: https://git.openjdk.org/jdk/pull/23104/files/9b59fb9e..2cbe4a27

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23104&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23104&range=00-01

  Stats: 56 lines in 2 files changed: 55 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/23104.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23104/head:pull/23104

PR: https://git.openjdk.org/jdk/pull/23104