[jdk20] RFR: 8298176: remove OpaqueZeroTripGuardPostLoop once main-loop disappears

[Emanuel Peter]

On Tue, 13 Dec 2022 19:23:08 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> We recently removed `Opaque2` nodes in [JDK-8294540](https://bugs.openjdk.org/browse/JDK-8294540). `Opaque2` nodes prevented some optimizations during loop-opts. The original idea was to prevent the use of both the un-incremented and incremented value of a loop phi after the loop, to reduce register pressure. But `Opaque2` also had the effect that the limit of the loop would not be optimized, which meant that the iv-value (entry value of phi) in post loop would never collapse (either to constant or TOP), but always remain a range. Now that `Opaque2` is gone, it can happen that when the main-loop disappears, the limit collapses. The zero-trip guard of the post-loop would be false, but does not collapse because of the `OpaqueZeroTripGuard`. The post-loop can half-collapse, leaving an inconsistent graph below the zero-trip guard if.
>> 
>> **Solution**
>> Have `OpaqueZeroTripGuardMainLoop` for main loop zero-trip guard, and `OpaqueZeroTripGuardPostLoop` for post-loop zero trip guard. Let `OpaqueZeroTripGuardPostLoop` remove itself once it cannot find the main-loop above it. We have these opaque nodes there to prevent collapsing of the zero-trip guards as long as the limits may still change, but after the main-loop is removed, no unrolling is done anymore, so the limit of the post-loop cannot change anymore, hence it is safe to remove the opaque node there.
>> 
>> An alternative approach was to let the main-loop remove the opaque node of the post-loop's zero-trip guard. But that does not work reliably, as the main-loop may get removed during PhaseCCP, and the main-loop is simply removed as "useless". Hence the LoopNode of the main-loop does not have a chance to detect its death during IGVN.
>
> "The zero-trip guard of the post-loop would be false, but does not collapse because of the OpaqueZeroTripGuard. "
> Can we just look only for this condition (guard is false) instead of looking through graph to see if main loop disappeared?

@vnkozlov @rwestrel I am now very unsure about this fix myself.
On the one hand, I and Christian do not understand why the OpaqueZeroTripGuardPostLoopNode would be there in the first place:

This opaque and the iv of the post-loop take their input from the Phi that merges main-loop exit-value and pre-loop exit value (if zero-trip-guard of main decides to not enter main).

1. If this Phi ever decays to a constant: We can never change iv for post-loop.
2. If this Phi ever is a range, but a range outside the trip-count of the post-loop, then the post-loop phi will detect this, and replace itself with top, and deconstruct the loop from the inside. But the OpaqueZeroTripGuardPostLoopNode still guards the zero-trip-guard from decaying. We get an inconsistent graph.

So it seems to me we either guard both the iv and the zero-trip-guard or nothing. So far we don't really understand why there is that opaque, it is there for 22 years. Completely removing it could be a solution, but it is unclear if that is safe. Maybe it does something we are not aware of.

After re-analyzing, I cannot find a case where the main LoopNode is removed during CCP, it all seems to be during IGVN. So I can try out the fix Roland suggested: when the main LoopNode detects its removal, let it find (graph-walk to) the zero-trip-guard of the post-loop,  and remove the opaque node there. I'll need to see how easy that is, given that different things could have decayed during IGVN already, so the graph-walk could be a bit tricky.

-------------

PR: https://git.openjdk.org/jdk20/pull/22