RFR: 8364757: Missing Store nodes caused by bad wiring in PhaseIdealLoop::insert_post_loop [v2]
Benoît Maillard
bmaillard at openjdk.org
Mon Sep 22 15:55:14 UTC 2025
> This PR introduces a fix for wrong results caused by missing `Store` nodes in C2 IR due to incorrect wiring in `PhaseIdealLoop::insert_post_loop`.
>
> ### Context
>
> The issue was initially found by the fuzzer. After some trial and error, and with the help of @chhagedorn I was able to reduce the reproducer to something very simple. After being compiled by C2, the execution of the following method led to the last statement (`x = 0`) to be ignored:
>
>
> static public void test() {
> x = 0;
> for (int i = 0; i < 20000; i++) {
> x += i;
> }
> x = 0;
> }
>
>
> After some investigation and discussions with @robcasloz and @chhagedorn, it appeared that this issue is linked to how safepoints are inserted into long running loops, causing the loop to be transformed into a nested loop with an `OuterStripMinedLoop` node. `Store` node are moved out of the inner loop when encountering this pattern, and the associated `Phi` nodes are removed in order to avoid inhibiting loop optimizations taking place later. This was initially adressed in [JDK-8356708](https://bugs.openjdk.org/browse/JDK-8356708) by making the necessary corrections in macro expansion. As explained in the next section, this is not enough here as macro expansion happens too late.
>
> This PR aims at addressing the specific case of the wrong wiring of `Store` nodes in _post_ loops, but on the longer term further investigations into the missing `Phi` node issue are necessary, as they are likely to cause other issues (cf. related JBS issues).
>
> ### Detailed Analysis
>
> In `PhaseIdealLoop::create_outer_strip_mined_loop`, a simple `CountedLoop` is turned into a nested loop with an `OuterStripMinedLoop`. The body of the initial loop remains in the inner loop, but the safepoint is moved to the outer loop. Later, we attempt to move `Store` nodes after the inner loop in `PhaseIdealLoop::try_move_store_after_loop`. When the `Store` node is moved to the outer loop, we also get rid of its input `Phi` node in order not to confuse loop optimizations happening later.
>
> This only becomes a problem in `PhaseIdealLoop::insert_post_loop`, where we clone the body of the inner/outer loop for the iterations remaining after unrolling. There, we use `Phi` nodes to do the necessary rewiring between the original body and the cloned one. Because we do not have `Phi` nodes for the moved `Store` nodes, their memory inputs may end up being incorrect.
>
> This is what the IR looks like after the creation of the post lo...
Benoît Maillard has updated the pull request incrementally with one additional commit since the last revision:
Improve comment about the is_member condition
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/27225/files
- new: https://git.openjdk.org/jdk/pull/27225/files/5142bbf0..0fc0be30
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=27225&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=27225&range=00-01
Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/27225.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/27225/head:pull/27225
PR: https://git.openjdk.org/jdk/pull/27225
More information about the hotspot-compiler-dev
mailing list