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

[Tobias Hartmann]

Thanks, Roland!

Sorry for the delay, I was busy with other work.

On 22.09.2015 15:04, Roland Westrelin wrote:
> 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. 

I also suspect that it initially wasn't robust enough. Seems like you did some changes with JDK-8005071 to make it work after IGVN:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2012-December/009266.html

However, I would prefer not to change the order of optimization phases here but it's probably worth to investigate.

> Maybe we could add an IfProjNode::Ideal method that disconnects the other branch of the If when this branch is always taken and that does so even during parsing. Given Ideal is called before Identity, that would guarantee the next call to Identity optimizes the If out.

As you suggested, I added an IfProjNode::Ideal that disconnects the never taken branch from the IfNode. The subsequent call to Identity then removes the IfNode:
http://cr.openjdk.java.net/~thartmann/8136469/webrev.03/

However, I wondered if this is "legal" because the comment in Node::ideal says:

 // The Ideal call almost arbitrarily reshape the graph rooted at the 'this'
 // pointer.

But we are changing the graph "above" the this pointer. I executed tests with -XX:+VerifyIterativeGVN and everything seems to work fine.
Another solution would be to cut the *current* branch if it is never taken:
http://cr.openjdk.java.net/~thartmann/8136469/webrev.02/

But this solution depends on the assumption that we execute the identity() of the other ProjNode which is not guaranteed by GVN (I think).

Therefore I would like to go for webrev.03. I verified that this solves the problem and tested the fix with JPRT.

Best,
Tobias