RFR: 8327963: C2: fix construction of memory graph around Initialize node to prevent incorrect execution if allocation is removed [v5]

Fri May 16 09:30:57 UTC 2025

On Thu, 15 May 2025 12:37:26 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> > Thanks for working on this, Roland! A "dumb" question: could the issue also be addressed by ensuring that dead allocations are removed earlier (e.g. in the call to `PhaseMacroExpand::eliminate_allocate_node` performed as part of escape analysis/scalar replacement, before loop optimizations)? It seems this would also prevent the miscompilations in `TestEliminationOfAllocationWithoutUse`, no?
> 
> I don't thing that would work (but haven't tried). The problem is that the memory graph is broken around `Initialize` nodes from the time they are added to the IR (parse time) and that's only exposed once they are removed from the graph. But I don't think it matters when they are removed.

I gave it a quick try (https://github.com/openjdk/jdk/commit/c28f81a7ef2a4f3d3cb761ea23a80c09276e7e58) and removing dead array allocations early (which are trivially detected as non-escaping but marked as non-scalar replaceable) is sufficient to fix all failures in `TestEliminationOfAllocationWithoutUse`. I have not tested the patch thoroughly and it might be that the dead test is too weak or I am missing something else, but this seems to me something worth exploring before committing to the solution proposed in this PR.

> What you suggest does also sound quite conservative: it could very well be that an allocation looses all its uses after some rounds of optimization but in the scheme you suggest, that allocation wouldn't be optimized out.

Note that I am not necessarily suggesting disabling "late" elimination of allocations at macro expansion. But it would be good, in light of the above findings, to find actual cases where the seemingly simpler alternative of removing dead allocations early is not sufficient for correctness, to motivate the more complex approach proposed in this PR.

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

PR Comment: https://git.openjdk.org/jdk/pull/24570#issuecomment-2886175661