[15] RFR(S): 8238765: PhaseCFG::schedule_pinned_nodes cannot handle precedence edges from unmatched CFG nodes correctly

[Christian Hagedorn]

Hi

Please review the following patch:
https://bugs.openjdk.java.net/browse/JDK-8238765
http://cr.openjdk.java.net/~chagedorn/8238765/webrev.00/

In a failing internal test case, GCM sometimes moved a load before the 
associated null check which resulted in a segfault. I don't think it's 
specific to ZGC - it just reveals the problem due to a specific IR shape 
which is more common with ZGC. Unfortunately, I was not able to extract 
a simpler test case from the failing internal test.

The problem could be traced back to the dominator check in 
PhaseCFG::schedule_pinned_nodes() with precedence edges [1]. At some 
point 'node->in(0)' (415 MachProj) is neither a block start nor a block 
projection and 'n' (409 IfTrue) is actually not dominating 'node->in(0)' 
[2]. But is_dominator(n, node->in(0)) returns true for the following 
reason: get_block_for_node(node->in(0)) returns null because 415 
MachProj is not matched yet (neither a block start nor a block 
projection). Therefore, d->dom_lca(n) == d is true since d->dom_lca(n) 
returns 'this' when 'n' is null. After GCM, the null check 414 testP_reg 
and its 413 jmpCon and 455 loadRange are now both in the same block and 
control dependent on 415 MachProj and can be executed in the wrong order 
[3].

The patch now processes all CFG precedence edges. This newly includes 
CFG nodes in the middle of a block (i.e. safepoints and control 
projections). In order to handle them, is_dominator() is extended to 
walk the control graph up in case a node is in the middle of a block and 
therefore not matched, yet, to find the associated block. If a 
precedence edge is not a CFG node then it must be an input to a StoreCM 
node.


Thank you!

Best regards,
Christian


[1] 
http://hg.openjdk.java.net/jdk/jdk/file/689d165369ac/src/hotspot/share/opto/gcm.cpp#l182
[2] Part of the IR of the failing internal test before GCM: 
https://bugs.openjdk.java.net/secure/attachment/86640/dominator_check.png
[3] Part of the IR of the failing internal test after GCM: 
https://bugs.openjdk.java.net/secure/attachment/86641/wrong_control_dependencies.png