RFR: 8364757: Missing Store nodes caused by bad wiring in PhaseIdealLoop::insert_post_loop [v3]

Benoît Maillard bmaillard at openjdk.org
Mon Sep 22 16:21:03 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:

  Use main_end instead of loop->tail()->in(0)

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/27225/files
  - new: https://git.openjdk.org/jdk/pull/27225/files/0fc0be30..f2eb376f

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

  Stats: 14 lines in 1 file changed: 0 ins; 2 del; 12 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