RFR: 8303045: Remove RegionNode::LoopStatus::NeverIrreducibleEntry assert with wrong assumption

Emanuel Peter epeter at openjdk.org
Mon Feb 27 07:18:20 UTC 2023


On Thu, 23 Feb 2023 10:43:50 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> I had to remove the first of these two asserts:
>> https://github.com/openjdk/jdk/blob/ac7119f0d5319a3fb44dc67a938c3e1eb21b9202/src/hotspot/share/opto/cfgnode.cpp#L438-L442
>> 
>> **Context**
>> 
>> The original idea for the two asserts was that we cannot accidentally lose information:
>> The default value is `NeverIrreducibleEntry`, and once we change it to a different value (`Reducible` or `MaybeIrreducibleEntry`) we have no reason to ever go back to the default.
>> 
>> However, for this, the second assert suffices: we should only set the value if it is still at default.
>> 
>> **Details: explanation of regression test**
>> 
>> I have an example where the first assert fails, it is the regression test that is attached here.
>> 
>> We inline
>> https://github.com/openjdk/jdk/blob/c6b4728177753513c87ee025a99f373e97de1466/test/hotspot/jtreg/compiler/loopopts/TestInlinedSplitFallInIrreducibleLoopStatusMain.java#L45-L48
>> 
>> And in the inlined method, we have a region on which `IdealLoopTree::split_fall_in` is called:
>> https://github.com/openjdk/jdk/blob/c6b4728177753513c87ee025a99f373e97de1466/test/hotspot/jtreg/compiler/loopopts/TestInlinedSplitFallInIrreducibleLoopStatus.jasm#L53-L54
>> 
>> Since the region is in an inlined method, its loop status is still `NeverIrreducibleEntry` (we can only set it to `Reducible` if we are absolutely sure there is no enclosing irreducible loop. And we only want to set `MaybeIrreducibleEntry` if we have to, because then we have to do additional work when an entry is lost).
>> Now, when we do `split_fall_in`, we copy the loop status from the `_head` region to the new `landing_pad`. So we essentially did `set_loop_status(NeverIrreducibleEntry)`, which triggered the assert.
>> https://github.com/openjdk/jdk/blob/c6b4728177753513c87ee025a99f373e97de1466/src/hotspot/share/opto/loopnode.cpp#L3144
>> 
>> ![image_480](https://user-images.githubusercontent.com/32593061/220640119-8fe59143-bd6e-42fe-8df0-1b01f1e87fb5.png)
>> Here the graph of the test, before we call `split_fall_in`. Red: `test_outer`. Orange: `test_inner`. Yellow: the loop inside `test_inner`, with `_head == 78 Region`.
>
> That sound reasonable. Looks good!

Thanks @chhagedorn @vnkozlov for the reviews!

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

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


More information about the hotspot-compiler-dev mailing list