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

[Damon Fenacci]

On Mon, 20 Oct 2025 05:56:03 GMT, Tobias Hartmann <thartmann 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.

Thanks for the review @TobiHartmann.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1675#issuecomment-3420877016