[14] 8235984: C2: assert(out->in(PhiNode::Region) == head || out->in(PhiNode::Region) == slow_head) failed: phi must be either part of the slow or the fast loop

Christian Hagedorn christian.hagedorn at oracle.com
Mon Dec 23 09:30:25 UTC 2019


Hi

Please review the following options for:
https://bugs.openjdk.java.net/browse/JDK-8235984

The problem:
The original fix for JDK-8233033 [1] assumed that partially peeled 
statements always have a dependency inside the loop to be unswitched 
(i.e. when following their outputs eventually a phi node is hit 
belonging to either the slow or the fast loop). However, that is not 
always the case. As a result the assert was hit.

I suggest the following options:

Option a) http://cr.openjdk.java.net/~chagedorn/8235984/webrev.a.00/
We first check at the start of loop unswitching if there are loop 
predicates for the loop to be unswitched and if they have an additional 
control dependency to partially peeled statements (outcnt > 1). Then we 
explicitly check the assumption that partially peeled statements have a 
dependency in the loop to be unswitched (in order to keep the fix for 
JDK-8233033). If that is not the case, we bailout. We could then file an 
RFE for JDK-15 to handle this missing case properly and remove the bailout.

Option b) http://cr.openjdk.java.net/~chagedorn/8235984/webrev.b.00/
We only check at the start of loop unswitching if there are loop 
predicates for the loop to be unswitched and if they have an additional 
control dependency to partially peeled statements (outcnt > 1). If 
that's the case we bailout without having the fix from JDK-8233033. We 
could then file an RFE for JDK-15 to properly handle partially peeled 
statements all together (kinda a REDO of the fix for JDK-8233033).

Option c)
Trying to fix the missing cases from JDK-8233033 for JDK-14 without a 
bailout.


I've tried to come up with a fix (option c) last week but without 
success so far. The idea was to also clone the partially peeled 
statements without a dependency in the loop to be unswitched, change 
their control input to the correct cloned predicates and then add a phi 
node on each loop exit, where we merge the slow and fast loop, to select 
the correct value. However, this has not worked properly, yet and also 
involves a higher risk due to its complexity. I think we should not 
target that option for JDK-14 but do it for JDK-15 in an RFE.

Thus, I'd opt for either option a) or b). I tested Tier 1-7 for the 
complete bailout b) and Tier 1-8 for the "conditioned" bailout a). Both 
look fine. I also ran some standard benchmarks comparing a) and b) to a 
baseline where I excluded the fix for JDK-8233033 (without bailing out 
and trying to fix the problem). I could not see any difference in 
performance. Therefore, it suggests to go with the low risk option b) 
for JDK-14 and do the entire fix in an RFE for JDK-15.

What do you think?


Thank you!

Best regards,
Christian


[1] https://bugs.openjdk.java.net/browse/JDK-8233033


More information about the hotspot-compiler-dev mailing list