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

[Roberto Castañeda Lozano]

On Tue, 27 May 2025 12:49:40 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.
>
> Daniel Skantz has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test comment

Thanks for addressing my comments, looks good!

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

Marked as reviewed by rcastanedalo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25461#pullrequestreview-2870981345