[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