RFR: 8263971: C2 crashes with SIGFPE with -XX:+StressGCM and -XX:+StressIGVN [v3]

[Tobias Hartmann]

On Thu, 25 Mar 2021 13:17:58 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> For the inner for loop of the test case:
>> 
>> 1- the control input of the DivI node in the loop body is cleared
>>   because the range of values for i23, the loop's iv, is known to
>>   never be null.
>> 
>> 2- pre/main/post loops are added
>> 
>> 3- the main loop is unrolled enough that it's fully unrolled but
>>   actually never entered
>> 
>> 4- because the main loop is no longer a looop, DiVI nodes from the
>> main loop can float and are scheduled in the pre loop.
>> 
>> 5- the main loop is never executed (the values of the iv for the main
>> loop after unrolling fall outside the (3, 68] range of values of the
>> initial loop), one of the main loop DivI nodes divides by 0 and
>> because it doesn't stay in the main loop, the crash occurs.
>> 
>> The calls to PhaseIdealLoop::cast_incr_before_loop() in
>> PhaseIdealLoop::insert_pre_post_loops() should guarantee that nodes
>> that uses the loop iv stay control dependent on the test that guards
>> the main loop. For that it inserts a CastII node with the
>> _carry_dependency flag set. With 8256730, that CastII for the main
>> loop is pushed through the iv AddI of the pre loop. In the process, it
>> looses the _carry_dependency flag which causes the CastII node to be
>> optimized out and the DivI nodes to float.
>> 
>> With the proposed fix, CastII nodes created by the logic from 8256730
>> have the _carry_dependency flag set if the CastII node that's
>> transform has it.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test with -XX:+IgnoreUnrecognizedVMOptions

Looks good to me.

test/hotspot/jtreg/compiler/loopopts/TestLostDependencyOnZeroTripGuard.java line 30:

> 28:  *
> 29:  * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileOnly=TestLostDependencyOnZeroTripGuard -XX:+UnlockDiagnosticVMOptions
> 30:  *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+StressGCM -XX:StressSeed=886771365 TestLostDependencyOnZeroTripGuard

Is it worth adding another @run with no fixed seed? Just to give this a chance to still trigger the bug if the implementation changes in the future such that the fixed seed does not work anymore.

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

Marked as reviewed by thartmann (Reviewer).

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