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

[Roland Westrelin]

On Fri, 3 Mar 2023 16:25:39 GMT, Vladimir Kozlov <kvn 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.

@vnkozlov @TobiHartmann thanks for the reviews

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

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