RFR: 8303564: C2: "Bad graph detected in build_loop_late" after a CMove is wrongly split thru phi

[Vladimir Kozlov]

On Fri, 3 Mar 2023 10:22:55 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The following steps lead to the crash:
> 
> - In `testHelper()`, the null and range checks for the `field1[0]`
>   load are hoisted out of the counted loop by loop predication
>   
> - As a result, the `field1[0]` load is also out of loop, control
>   dependent on a predicate
>   
> - pre/main/post loops are created, the main loop is unrolled, the `f`
>   value that's stored in `field3` is a Phi that merges the values out
>   of the 3 loops.
> 
> - the `stop` variable that captures the limit of the loop is
>   transformed into a `Phi` that merges 1 and 2.
>   
> - As a result, the Phi that's stored in `field3` now only merges the
>   value of the pre and post loop and is transformed into a `CmoveI`
>   that merges 2 values dependent on the `field1[0]` `LoadI` that's
>   control dependent on a predicate.
>   
> - On the next round of loop opts, the `CmoveI` is assigned control
>   below the predicate but the `Bool`/`CmpI` for the `CmoveI` is
>   assigned control above, right below a `Region` that has a `Phi` that
>   is input to the `CmpI`. The reason is this logic: 
>   https://github.com/rwestrel/jdk/blob/99f5687eb192b249a4a4533578f56b131fb8f234/src/hotspot/share/opto/loopnode.cpp#L5968
>   
> - The `CmoveI` is split thru phi because the `Bool`/`CmpI` have
>   control right below a `Region`. That shouldn't happen because the
>   `CmoveI` itself doesn't have control at the `Region` and is actually
>   pinned through the `LoadI` below the `Region`.
>   
> The fix I propose is to check the control of the `CmoveI` before
> proceding with split thru phi.

Good.

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

Marked as reviewed by kvn (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12851