8357105: C2: compilation fails with "assert(false) failed: empty program detected during loop optimization" #25395

danielogh · 2025-05-22T15:19:08Z

This pull request contains a fix for JDK-8357105.

The problem is performing stacked string concatenation optimization between a pair of StringBuilder.append().toString()-links SB1 and SB2, where the parameter of an append call in SB2 has a complex dependency on the result of SB1, which in turn is replaced by top() during stringopts -- similar to JDK-8271341, which had a diamond if-structure using the result of SB1, while in this case the use is an unstable If. In the attached regression test, a live part of the graph gets optimized away during later phases and ultimately the whole graph vanishes.

The proposed solution is to simply exclude this specific case. This bug has existed for a long time and stacked concats is a niche optimization.

Testing:
Tier1-4.

Extra testing:
Ran Tier1-4 with an instrumented build and observed that we do not disable stacked concatenation in any previously known case after the fix.

Progress

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

Issue

JDK-8357105: C2: compilation fails with "assert(false) failed: empty program detected during loop optimization" (Bug - P3)

Reviewers

SendaoYan (@sendaoYan - Committer)
Roberto Castañeda Lozano (@robcasloz - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25395

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25395.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-05-22T15:19:52Z

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

openjdk · 2025-05-22T15:20:34Z

@danielogh 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:

8357105: C2: compilation fails with "assert(false) failed: empty program detected during loop optimization"

Reviewed-by: syan, rcastanedalo

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 50 new commits pushed to the master branch:

ed4cd2a: 8356989: Unexpected null in C2 compiled code
9946c85: 8357621: G1: Clean up G1BiasedArray
5cdeef8: 8331859: [PPC64] Remove support for Power7 and older
... and 47 more: https://git.openjdk.org/jdk/compare/1e57648abd569295f42dc19c00edfcc90c00b1d3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

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

openjdk · 2025-05-22T15:21:06Z

@danielogh The following label will be automatically applied to this pull request:

hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-05-22T15:24:30Z

Webrevs

sendaoYan · 2025-05-23T02:36:45Z

test/hotspot/jtreg/compiler/stringopts/TestStackedConcatsAppendUncommonTrap.java

+public class TestStackedConcatsAppendUncommonTrap {
+
+    public static void main (String... args) {
+        for (int i = 0; i < 1_000_000; i ++) {


How about i ++ replaced as i++. The whitespace seems do not need.

sendaoYan · 2025-05-23T02:42:28Z

test/hotspot/jtreg/compiler/stringopts/TestStackedConcatsAppendUncommonTrap.java

+
+    public static void main (String... args) {
+        for (int i = 0; i < 1_000_000; i ++) {
+            f(" ");


Should we use the function return value, to avoid the compiler do the dead code elimination optimization

robcasloz

The overall analysis and fix look good to me, I just have some minor style, test, and code comment suggestions.

src/hotspot/share/opto/stringopts.cpp

robcasloz · 2025-05-23T15:04:56Z

test/hotspot/jtreg/compiler/stringopts/TestStackedConcatsAppendUncommonTrap.java

+ * @summary Test stacked string concatenations where the toString result
+ *          of the first StringBuilder chain is wired into an uncommon trap
+ *          located in the second one.
+ * @run main/othervm compiler.stringopts.TestStackedConcatsAppendUncommonTrap


Please add a second run that is constrained via JVM flags to be more stable and easier to analyze, using -Xbatch, -XX:CompileOnly=..., and perhaps -XX:-TieredCompilation. Using -Xbatch also allows you to reduce the number of warm-up iterations, many tests use 10_000.

Co-authored-by: Roberto Castañeda Lozano <[email protected]>

sendaoYan

LGTM

robcasloz

Looks good, thanks! Please re-run testing of your latest changes (if you haven't yet) before integration.

danielogh · 2025-05-26T14:21:00Z

Thanks for the reviews and suggestions!

danielogh · 2025-05-26T14:21:05Z

/integrate

openjdk · 2025-05-26T14:22:17Z

Going to push as commit a300c35.
Since your change was applied there have been 54 commits pushed to the master branch:

bd09589: 8356632: Fix remaining {@link/@linkplain} tags with refer to private/protected types in java.base
daa8eda: 8357056: RISC-V: Asm fixes - load/store width
7002233: 8351950: C2: AVX512 vector assembler routines causing SIGFPE / no valid evex tuple_table entry
... and 51 more: https://git.openjdk.org/jdk/compare/1e57648abd569295f42dc19c00edfcc90c00b1d3...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-05-26T14:22:28Z

@danielogh Pushed as commit a300c35.

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

fix

1ed6dde

openjdk bot added the rfr Pull request is ready for review label May 22, 2025

openjdk bot added the hotspot-compiler [email protected] label May 22, 2025

sendaoYan suggested changes May 23, 2025

View reviewed changes

robcasloz suggested changes May 23, 2025

View reviewed changes

danielogh and others added 3 commits May 26, 2025 08:04

Update src/hotspot/share/opto/stringopts.cpp

41b8b6b

Co-authored-by: Roberto Castañeda Lozano <[email protected]>

Apply suggestions from code review

65c331f

Co-authored-by: Roberto Castañeda Lozano <[email protected]>

add second run; - iteration count; add use of result

a4996f6

sendaoYan approved these changes May 26, 2025

View reviewed changes

robcasloz approved these changes May 26, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 26, 2025

openjdk bot added the integrated Pull request has been integrated label May 26, 2025

openjdk bot closed this May 26, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 26, 2025

8357105: C2: compilation fails with "assert(false) failed: empty program detected during loop optimization" #25395

8357105: C2: compilation fails with "assert(false) failed: empty program detected during loop optimization" #25395

Uh oh!

Conversation

danielogh commented May 22, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented May 22, 2025

Uh oh!

openjdk bot commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented May 22, 2025

Uh oh!

mlbridge bot commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

sendaoYan May 23, 2025

Choose a reason for hiding this comment

Uh oh!

sendaoYan May 23, 2025

Choose a reason for hiding this comment

Uh oh!

robcasloz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robcasloz May 23, 2025

Choose a reason for hiding this comment

Uh oh!

sendaoYan left a comment

Choose a reason for hiding this comment

Uh oh!

robcasloz left a comment

Choose a reason for hiding this comment

Uh oh!

danielogh commented May 26, 2025

Uh oh!

danielogh commented May 26, 2025

Uh oh!

openjdk bot commented May 26, 2025

Uh oh!

openjdk bot commented May 26, 2025

Uh oh!

Uh oh!

danielogh commented May 22, 2025 •

edited by openjdk bot

Loading

openjdk bot commented May 22, 2025 •

edited

Loading

mlbridge bot commented May 22, 2025 •

edited

Loading