RFR: 8263189: C2: assert(!had_error) failed: bad dominance

[Roland Westrelin]

On Tue, 23 Mar 2021 07:05:28 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> A long counted loop has a Phi that depends on the iv
>> Phi. PhaseIdealLoop::is_counted_loop() is able to compute a narrow
>> type for the iv Phi (the loop only runs for a few iterations). As a
>> consequence the Phi that depends on the iv Phi also gets a narrow
>> type. When the long counted loop is transformed into a loop nest, even
>> though the long loop is known to run for only a few iterations, the
>> inner int counted loop that's created gets a wide type: the narrow
>> type of the long counted loop iv Phi should result in a narrow type
>> for the int counted loop iv Phi but doesn't. C2 then creates
>> pre/main/post loops for the inner int counted loop. It wouldn't have
>> proceeded with this step, had the number of iterations of the loop not
>> been lost. In the process, it creates Phi to merge the values from the
>> pre/main/post loop. Two of those Phis are for the dependent Phi, the
>> one that captured the narrow type. The type of one of the 2 Phis
>> becomes top because we get in an impossible situation where the loop
>> that's known to have very few iterations was cloned twice. One of the
>> just created Phi becomes top and dominance is broken.
>> 
>> The reason the iv Phi narrow type is lost at long loop transformation
>> time is because it's queried from the igvn but it was not yet recorded
>> with the igvn and instead set when the long counted loop was created
>> in the same loop pass. The fix for that is to query the type stored in
>> the Phi rather than from the igvn.
>> 
>> Now it's not right that we end up with a dependent Phi that captures
>> the narrow type (which happens at igvn time) before the loop is even
>> created (I said that happens in the same loop opts pass so igvn has
>> not had a chance to run). The reason for that is that the same loop is
>> transformed multiple times:
>> 
>> 1- the long counted loop is created
>> 
>> 2- the long counted loop is transformed to a loop nest with an inner
>>   int counted loop. The outer loop is transformed back to a regular
>>   loop.
>> 
>> 3- the inner int counted loop is found to be empty and so optimized out
>> 
>> 4- the long counted loop for the same loop is created again
>> 
>> 5- a loop nest is created again
>> 
>> This looks wrong and I added code to prevent retransforming a long
>> counted loop.
>
> Nice analysis. Looks good to me too.

@TobiHartmann thanks for the review

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

PR: https://git.openjdk.java.net/jdk/pull/3005