[16] RFR(M): 8249607: C2: assert(!had_error) failed: bad dominance

[Christian Hagedorn]

Hi

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

In the testcase, a LoadSNode is cloned in 
PhaseIdealLoop::split_if_with_blocks_post() for each use such that they 
can float out of a loop. To ensure that these loads cannot float back 
into the loop, we pin them by setting their control input [1]. In the 
testcase, all 3 new clones are pinned to a loop exit node that is part 
of an outer strip mined loop (see [2]).

The clones LoadS 901 and 902 have a late control that is outside of the 
strip mined loop 879. But the dominance information is still correct 
after the SplitIf optimization since the inner loop exit node 876 
IfFalse is still on the dominator chains of these late controls.

We later create pre/main/post loops and add additional RegionNodes to 
merge them together. However, we do not consider these LoadSNodes that 
have a control input from 876 IfFalse. When later verifying for each 
node that its early control dominates its latest possible control, we 
fail because we cannot reach 876 IfFalse anymore on a dominator chain 
for the late controls of LoadS 901 and 902 which start further down 
outside of the strip mined loop 879.

We have two options to fix this. We could either update the wrong 
control inputs from 876 IfFalse during the creation/merging of 
pre/main/post loops or directly fix it inside 
split_if_with_blocks_post(). I think it is makes more sense and is also 
easier to directly fix it in split_if_with_blocks_post() where we could 
be less pessimistic when pinning loads.

The fix now checks if late_load_ctrl is a loop exit of a loop that has 
an outer strip mined loop and if it dominates x_ctrl. If that is the 
case, we use the outer loop exit control instead. This also means that 
the loads can completely float out of the outer strip mined loop. 
Applying that to the testcase, we get [3] instead of [2]. LoadS 901 and 
902 are both at the outer strip mined loop exit while 903 LoadS is still 
at the inner loop due to 575 StoreI (x_ctrl is 876 IfFalse and dominates 
the outer strip mined loop exit). The process of creating pre/main/post 
loops will then take care of these control inputs of the LoadSNodes and 
rewires them to the newly created RegionNode such that the dominator 
information is correct again.

I additionally updated the printing output in case of such a dominance 
failure which I think improves the analysis of these problems. It now 
also prints the idom chain of the early node and the actual real LCA of 
early and the wrong LCA together with the idom index:

Real LCA of early 876 (idom[5]) and (wrong) LCA 728 (idom[19]):
  1052	If	===  523  1051  [[ 1035  1053 ]] P=0.999999, C=-1.000000


Thank you!

Best regards,
Christian


[1] 
http://hg.openjdk.java.net/jdk/jdk/file/1c332a041243/src/hotspot/share/opto/loopopts.cpp#l1456
[2] 
https://bugs.openjdk.java.net/secure/attachment/89911/pinned_at_inner_loop_exit.png
[3] 
https://bugs.openjdk.java.net/secure/attachment/89912/pinned_at_outer_strip_mined_loop_exit.png