RFR: 8324345: Stack overflow during C2 compilation when splitting memory phi

[Tobias Hartmann]

On Thu, 18 Jul 2024 15:24:33 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

> `ConnectionGraph::find_inst_mem` contains recursive calls that can lead to a native C++ stack overflow in some cases.
> 
> ### Changeset
> 
> - Add a simple recursion depth limit to `ConnectionGraph::find_inst_mem`. When `find_inst_mem` reaches the limit, we bail out from the current compilation and retry without escape analysis. The depth limit is currently set to a fixed value of 1000, based on ad hoc experiments. We want a small enough limit to ensure we never get a stack overflow, but still high enough to allow completing the escape analysis in most compilations. I'm open to other suggestions for setting the limit (perhaps adaptively based on the execution environment?).
> - Add a regression test `TestFindInstMemRecursion.java`.
> - Temporarily remove the implicit memory limit (due to [JDK-8331185](https://bugs.openjdk.org/browse/JDK-8331185)) from `VectorReplicateLongSpecialImmTest.java`. The memory limit triggers after this changeset, but the (potential) problem is most likely unrelated to this issue. I will create a new issue after merging this PR which we can look into separately.
> 
> ### Testing
> 
> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/9976490176)
> - tier1 to tier4 on Windows x64, Linux x64, Linux aarch64, macOS x64, and macOS aarch64.
> - Standard performance benchmarking. I found no regressions with the proposed depth limit of 1000.

test/hotspot/jtreg/compiler/escapeAnalysis/TestFindInstMemRecursion.java line 30:

> 28:  *          overflow.
> 29:  *
> 30:  * @run main/othervm -Xcomp -XX:CompileThreshold=10 -XX:-TieredCompilation

This will lead to **a lot** of compilations and therefore increase the runtime of the test. I assume there is only one problematic method, so couldn't you limit compilation to that one?

test/hotspot/jtreg/compiler/vectorapi/VectorReplicateLongSpecialImmTest.java line 39:

> 37:  * @requires os.arch == "aarch64"
> 38:  * @modules jdk.incubator.vector
> 39:  * @run testng/othervm -XX:UseSVE=0 -XX:-TieredCompilation -XX:CompileThreshold=100 -XX:+IgnoreUnrecognizedVMOptions -XX:CompileCommand=MemLimit,*.*,0 compiler.vectorapi.VectorReplicateLongSpecialImmTest

Does that mean we hit the bailout here and re-compilations hits the memory limit?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20238#discussion_r1684196124
PR Review Comment: https://git.openjdk.org/jdk/pull/20238#discussion_r1684197938