[lworld] RFR: 8367405: [lworld] C2 compilation fails with empty program detected during loop optimization

[Tobias Hartmann]

On Mon, 13 Oct 2025 15:38:11 GMT, Damon Fenacci <dfenacci at openjdk.org> wrote:

> ## Issue
> The symptom is that an empty program is detected during loop optimization when running `compiler/stringopts/TestStackedConcatsAppendUncommonTrap.java` with `-XX:-TieredCompilation -XX:+StressUnstableIfTraps`.
> 
> ## Cause
> The cause's origin is string concatenation failing to bail out. More specifically, at the end of `StringConcat::validate_control_flow`, while validating the results, we miss processing a `Phi` node and wrongly let the validation pass. The starting point is this:
> 
> <img width="938" height="820" alt="image" src="https://github.com/user-attachments/assets/b7a3ee17-33d7-4ecb-8fb0-fb962a831f04" />
> 
> While processing the `Proj` node and going through its outputs, we reach the `CheckCastPP` and add its uses to the worklist (basically only the `Phi` node).
> 
> https://github.com/openjdk/valhalla/blob/708b4f92431df90c115dac840fb8194ec3aac3fe/src/hotspot/share/opto/stringopts.cpp#L1138-L1145
> 
> Surprisingly we don't add the `CheckCastPP` node itself. 
> In one of the next worklist iterations we process the `Phi` node and its uses and none of them make the validation fail.
> 
> Side note: `CheckCastPP` is added as additional profiling in Valhalla when parsing `acmp`
> https://github.com/openjdk/valhalla/blob/23dc125e5791238a97923108b77c6fc0b59d894f/src/hotspot/share/opto/parse2.cpp#L2098-L2102
> 
> On the other hand in mainline `CheckCastPP` is not added and when processing the `Proj`, `Phi` is encountered as one of its uses and the validation fails straight away.
> 
> ## Fix
> 
> Unfortunately this looks yet like another patch in the stacked optimisation's pattern matching "denylist" approach. As mentioned by @danielogh and @robcasloz in the JBS issue, a long-term better approach would be to limit the concatenation to known safe cases.
> 
> While waiting for that, a reasonable ad-hoc solution for this Valhalla case is to add a specific `CheckCastPP` case to the validation of uses that also adds itself to the worklist. This will include `CheckCastPP` nodes added by Valhalla's additional profiling (in all fairness this doesn't exclude potential other origins, but the extend and outcome in terms of concatenation optimisation failures should be quite limited).
> Note: `CastPP` has to be treated differently: it seems that not adding itself in the worklist was done on purpose for `CastPP` to skip string null check `Phi` nodes among other things.
> 
> ## Testing
> 
> Tests: tier 1-3

Thanks for the detailed analysis Damon! The fix looks good to me.

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

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1675#pullrequestreview-3355093565