[9] RFR(S): 8136469: OptimizeStringConcat fails on pre-sized StringBuilder shapes

[Roland Westrelin]

> please review the following patch.
> 
> https://bugs.openjdk.java.net/browse/JDK-8136469
> http://cr.openjdk.java.net/~thartmann/8136469/webrev.00/
> 
> Problem:
> When creating a pre-sized StringBuilder, C2's string concatenation optimization sometimes fails to optimize the chain (see [1]). The problem is that the initial size of the StringBuilder depends on a static final boolean that is initialized to true at runtime. Therefore the string concatenation control flow chain [2] contains an IfNode with a ConI (1) as input instead of the expected BoolNode and StringConcat::validate_control_flow() silently bails out.
> 
> Solution:
> I changed the implementation to skip dead tests as they would be removed by IGVN later anyway. I added an assert to make sure we don't bail out silently if the input of the IfNode is not a bool. I also had to change validate_mem_flow() to handle dead ifs. Further, the assert in line 825 is unnecessary because we execute the same check in as_If().

This happens because PhaseStringOpts is performed right after parsing and we don’t optimize the test out in GVN. We don’t optimize the test out in GVN because IfProjNode::Identity() has an in(0)->outcnt() == 1 check that fails. That check is here so we don’t build a bad graph during IGVN but during GVN that extra check is not needed. We could optimize the If out. Except in Identity we don’t know whether we’re called from GVN or IGVN. We would know in Ideal but we can’t return in(0)->in(0), an old node, from Ideal. That’s frustrating.

So why don’t we run PhaseStringOpts after IGVN? We do that for incremental inlining so we know the PhaseStringOpts code is robust enough to be run after IGVN, now. Maybe it wasn’t initially. 

Roland.