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

[Roland Westrelin]

>> 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.

I thought about this more and I don’t think either work ok.

The problem with webrev.02 is that depending on the order the projection nodes are allocated and transformed, the optimization may not happened:

Node* never_taken = new IfTrueNode(..);
Node* always_taken = new IfFalseNode(..);
always_taken = gvn.transform(always_taken);
never_taken = gvn.transform(never_taken);

The problem with webrev.03 is that we may change a node that is not yet transformed (never_taken changed by call to gvn.transform(always_taken)). Not sure if it could break existing code but it’s clearly an unexpected behavior.

An other way would be to remove the in(0)->outcnt() == 1 check from IfProjNode::Identity() and in an IfProjNode::Ideal method do what you do in webrev.03 but when can_reshape is true only.

Roland.