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

[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 PhiNode to have top as an input, so our fix is to simply to adding top as the input to the phi node in the case it is found in `PhaseMacroExpand::value_from_mem_phi`.

After all macro eliminations another IGVN run occurs. In this run, the catch path with 462 MemBarReleaseLock dies and the Top propagates down into the Region node belonging to the phi node into which we stored top. Through idealization this path (and thus input) is removed from the RegionNode and PhiNode.

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

Commit messages:
 - Fix

Changes: https://git.openjdk.org/jdk/pull/23104/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23104&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8325030
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 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