RFR: 8336729: C2: Div/Mod nodes without zero check could be split through iv phi of outer loop of long counted loop nest resulting in SIGFPE [v3]

[Christian Hagedorn]

On Thu, 15 Aug 2024 12:13:20 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> The previous fix for preventing `Div/Mod` nodes to be split through iv phis when their divisor could become zero (see [JDK-8299259](https://bugs.openjdk.org/browse/JDK-8299259)) is not complete. Originally, we thought that it is enough to prevent a split through the phi if it belongs to a `BaseCountedLoop`:
>> https://github.com/openjdk/jdk/blob/da7311bbe37c2b9632b117d52a77c659047820b7/src/hotspot/share/opto/loopopts.cpp#L302-L304
>> 
>> The reasoning behind this was that the iv phi type of `BaseCountedLoops` can be improved in such a way that it does not contain zero but the backedge input can include zero:
>> https://github.com/openjdk/jdk/blob/da7311bbe37c2b9632b117d52a77c659047820b7/test/hotspot/jtreg/compiler/splitif/TestSplitDivisionThroughPhi.java#L69-L78
>> 
>> This optimization is not possible for `LoopNodes` where we know nothing about the number of iterations. 
>> 
>> However, there was an oversight: A `LongCountedLoop` is later split into an inner and an outer `LoopNode` where the inner loop is transformed into a `CountedLoop` while the outer loop stays a `LoopNode`. Both loops share the same optimized iv phi type as the original `LongCountedLoop`! Therefore, we can have the same situation as fixed in JDK-8299259 but with a `LoopNode` instead of a `CountedLoopNode` (see https://github.com/openjdk/jdk/pull/11900 for more details).
>> 
>> The simplest way to fix this is to extend the bailout fix of JDK-8299259 to not only apply to `BaseCountedLoopNodes` but to `LoopNodes` in general which is what I propose with this patch.
>> 
>> Thanks to @eme64 for extracting the original simpler reproducer to reproduce this problem easier. I've added an even simpler non-jasm reproducer in this patch in favor of the original jasm reproducer.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix number

Thanks Vladimir for your review!

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

PR Comment: https://git.openjdk.org/jdk/pull/20594#issuecomment-2293344903