8386676: [lworld] Infinite loop when pushing inline types down though phis during IGVN by rwestrel · Pull Request #2600 · openjdk/valhalla

rwestrel · 2026-06-30T12:15:26Z

We have the following subgraph

48  ConP  === 0
61253  Phi  === 228 _ 48
61252  InlineType  === _ 61253 61254 61255 61256
61248  Phi  === 242 61252 61253

Phi 61248 is transformed and PushInlineTypeDown::do_transform runs.

A new value type is created:

 61259  Phi  === 242 1 1
 61258  InlineType  === _ 61259 61260 61261 61262

First input of Phi 61248 is processed:

61259  Phi  === 242 61253 1
61258  InlineType  === _ 61259 61260 61261 61262

The second input of the Phi 61248, Phi 61253, is processed which
causes a recursive call to PushInlineTypeDown::do_transform. A value
type is created:

61264  Phi  === 228 _ 1
61263  InlineType  === _ 61264 61265 61266 61267

In the uses of Phi 61253, Phi 61253 is replaced by InlineType
61263.

One such as use isPhi 61259. As a consequence, the value type of the
caller of this PushInlineTypeDown::do_transform is modified:

 61259  Phi  === 242 61263 1
 61258  InlineType  === _ 61259 61260 61261 61262

PushInlineTypeDown::do_transform for Phi 61253 runs to completion:

61264  Phi  === 228 _ 48
61263  InlineType  === _ 61264 61265 61266 61267

PushInlineTypeDown::do_transform for Phi 61248 runs to completion:

48  ConP  === 0
61264  Phi  === 228 _ 48
61263  InlineType  === _ 61264 61265 61266 61267
61259  Phi  === 242 61263 61264
61258  InlineType  === _ 61259 61260 61261 61262

Which is a shape identical to what PushInlineTypeDown::do_transform
started from. Transformation then happens with Phi 61259 which
produces the same shape etc.

Things go wrong, I think, because of this code in
PushInlineTypeDown::do_transform:

if (_can_reshape) {
  // Replace phi right away to be able to use the inline
  // type node when reaching the phi again through data loops.
  PhaseIterGVN* igvn = _phase->is_IterGVN();
  for (DUIterator_Fast imax, i = phi->fast_outs(imax); i < imax; i++) {
    Node* u = phi->fast_out(i);
    igvn->rehash_node_delayed(u);
    imax -= u->replace_edge(phi, vt);
    --i;
  }
  igvn->rehash_node_delayed(phi);
  assert(phi->outcnt() == 0, "should be dead now");
}

because it changes an already processed input of a Phi and adds an
InlineType where there was none. From the comment, the goal is to
handle cases where we run into that same Phi when
PushInlineTypeDown::do_transform continues execution and not create
a new InlineType node. To do that, I propose recording that vt is
created for phi rather than changing the graph.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8386676: [lworld] Infinite loop when pushing inline types down though phis during IGVN (Bug - P4)

Reviewers

Tobias Hartmann (@TobiHartmann - Committer)

Contributors

Christian Hagedorn <chagedorn@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2600/head:pull/2600
$ git checkout pull/2600

Update a local copy of the PR:
$ git checkout pull/2600
$ git pull https://git.openjdk.org/valhalla.git pull/2600/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2600

View PR using the GUI difftool:
$ git pr show -t 2600

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2600.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-06-30T12:17:26Z

👋 Welcome back roland! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-06-30T12:18:15Z

@rwestrel This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8386676: [lworld] Infinite loop when pushing inline types down though phis during IGVN

Co-authored-by: Christian Hagedorn <chagedorn@openjdk.org>
Reviewed-by: thartmann

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the lworld branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

openjdk · 2026-06-30T12:18:41Z

@rwestrel this pull request can not be integrated into lworld due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8386676
git fetch https://git.openjdk.org/valhalla.git lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push

rwestrel · 2026-06-30T12:57:25Z

/template append

openjdk · 2026-06-30T13:00:24Z

@rwestrel The pull request template has been appended to the pull request body

mlbridge · 2026-06-30T14:18:34Z

Webrevs

01: Full - Incremental (350dc74e)
00: Full (521e7d92)

TobiHartmann · 2026-06-30T14:25:58Z

+ * @enablePreview
+ * @modules java.base/jdk.internal.value
+ *          java.base/jdk.internal.vm.annotation
+ * @run main/othervm -XX:+StressIGVN -XX:CompileCommand=compileonly,${test.main.class}::test -Xbatch


I think this requires -XX:+UnlockDiagnosticVMOptions, right?

TobiHartmann · 2026-06-30T14:27:03Z

+ * @run main/othervm -XX:+StressIGVN -XX:CompileCommand=compileonly,${test.main.class}::test -Xbatch
+ *                   -XX:-UseOnStackReplacement -XX:StressSeed=3696073068 -XX:CompileTaskTimeout=8000 ${test.main.class}
+ * @run main/othervm -XX:+StressIGVN -XX:CompileCommand=compileonly,${test.main.class}::test -Xbatch
+ *                   -XX:-UseOnStackReplacement -XX:CompileTaskTimeout=8000 ${test.main.class}


And CompileTaskTimeout is debug only.

rwestrel · 2026-06-30T14:31:41Z

@TobiHartmann thanks for having a look. Both should be fixed now.

TobiHartmann

The fix looks good to me. I'll run testing and report back once it finished.

rwestrel · 2026-06-30T15:09:11Z

/contributor add @chhagedorn

rwestrel · 2026-06-30T15:09:32Z

Thanks for the test case @chhagedorn

openjdk · 2026-06-30T15:12:00Z

@rwestrel
Contributor Christian Hagedorn <chagedorn@openjdk.org> successfully added.

TobiHartmann · 2026-07-01T04:57:49Z

Testing all passed. Ship it!

rwestrel · 2026-07-01T07:44:21Z

@TobiHartmann thanks for the reviews and testing

rwestrel · 2026-07-01T07:44:26Z

/integrate

openjdk · 2026-07-01T07:47:05Z

Going to push as commit dc8b7c5.
Since your change was applied there have been 5 commits pushed to the lworld branch:

43153f5: 8387488: [lworld] C2: acmp expansion fails with assert(replace != nullptr) failed: must succeed
285c9fc: 8387564: [lworld] cherry-pick JDK-8387554 ProblemList vmTestbase/nsk/jvmti/unit/functions/Dispose/JvmtiTest/TestDescription.java in virtual thread configs
50d7001: 8386719: [lworld] Remaining ValueClass argument verification issues
... and 2 more: https://git.openjdk.org/valhalla/compare/2ae8e47da016bb4d7718e7923c964c2b91c638e6...lworld

Your commit was automatically rebased without conflicts.

openjdk · 2026-07-01T07:47:13Z

@rwestrel Pushed as commit dc8b7c5.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

rwestrel added 3 commits June 30, 2026 12:07

more

528b2b2

more

0bfbe84

more

129b058

openjdk Bot added the merge-conflict Pull request has merge conflict with target branch label Jun 30, 2026

Merge branch 'lworld' into JDK-8386676

5299ce2

openjdk Bot removed the merge-conflict Pull request has merge conflict with target branch label Jun 30, 2026

whitespaces

521e7d9

openjdk Bot added the rfr Pull request is ready for review label Jun 30, 2026

TobiHartmann reviewed Jun 30, 2026

View reviewed changes

rwestrel added 2 commits June 30, 2026 16:27

review

6c1e39e

review

350dc74

TobiHartmann approved these changes Jun 30, 2026

View reviewed changes

openjdk Bot added the ready Pull request is ready to be integrated label Jun 30, 2026

openjdk Bot added the integrated Pull request has been integrated label Jul 1, 2026

openjdk Bot closed this Jul 1, 2026

openjdk Bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 1, 2026

Uh oh!

Conversation

rwestrel commented Jun 30, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper Bot commented Jun 30, 2026

Uh oh!

openjdk Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk Bot commented Jun 30, 2026

Uh oh!

rwestrel commented Jun 30, 2026

Uh oh!

openjdk Bot commented Jun 30, 2026

Uh oh!

mlbridge Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

TobiHartmann Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

TobiHartmann Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rwestrel commented Jun 30, 2026

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

rwestrel commented Jun 30, 2026

Uh oh!

rwestrel commented Jun 30, 2026

Uh oh!

openjdk Bot commented Jun 30, 2026

Uh oh!

TobiHartmann commented Jul 1, 2026

Uh oh!

rwestrel commented Jul 1, 2026

Uh oh!

rwestrel commented Jul 1, 2026

Uh oh!

openjdk Bot commented Jul 1, 2026

Uh oh!

openjdk Bot commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

rwestrel commented Jun 30, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented Jun 30, 2026 •

edited

Loading

mlbridge Bot commented Jun 30, 2026 •

edited

Loading