RFR: 8356246: C2: Compilation fails with "assert(bol->is_Bool()) failed: unexpected if shape" in StringConcat::eliminate_unneeded_control

[Roberto Castañeda Lozano]

On Tue, 27 May 2025 07:58:22 GMT, Daniel Skantz <dskantz at openjdk.org> wrote:

>> This pull request contains a fix for JDK-8356246.
>> 
>> During stacked concatenations, a pair of `StringBuilder.append().toString()` links SB and SB2 could have a diamond if structure `(Region -> <IfTrue, IfFalse> -> If)` created by String.valueOf that depends on the return value of SB1, which is going away (replaced by top() in `eliminate_call` in stringopts).
>> 
>> JDK-8271341 added folding of the region of the diamond-if to stringopts to avoid the case where a live part of the graph becomes unreachable as this top() propagates through the graph too quickly.
>> 
>> JDK-8291775 was a follow-up fix and instead used a constant test as input to the diamond If, as a case was discovered where the If was processed before the Region leading to a broken graph.
>> 
>> The code in JDK-8271341 assumes that the input to the If is a boolean, not a constant. If two diamond if-region structures in the same StringBuilder candidate share the same test, the second iteration in `eliminate_unneeded_control` will fail with an unexpected input. The proposed solution is to skip over the second iteration as the test has already been replaced by a constant -- both structures will be simplified independently during IGVN.
>> 
>> Testing:
>> T1-4.
>
> test/hotspot/jtreg/compiler/stringopts/TestStackedConcatsSharedTest.java line 30:
> 
>> 28:  *          is used as a shared test by two diamond Ifs in the second StringBuilder.
>> 29:  * @run main/othervm compiler.stringopts.TestStackedConcatsSharedTest
>> 30:  * @run main/othervm -Xbatch -XX:-TieredCompilation -Xcomp
> 
> Using -Xcomp for the test. Warming up with many iterations invalidated the optimization due to an unstable If.

Please add this information as a comment somewhere in the test file. Perhaps as a comment above the `String.valueOf(s)` calls, where the stable if speculation happens.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25461#discussion_r2109000022