RFR: 8303045: Remove RegionNode::LoopStatus::NeverIrreducibleEntry assert with wrong assumption
Christian Hagedorn
chagedorn at openjdk.org
Thu Feb 23 10:47:04 UTC 2023
On Wed, 22 Feb 2023 13:35:35 GMT, Emanuel Peter <epeter 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!
-------------
Marked as reviewed by chagedorn (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12713
More information about the hotspot-compiler-dev
mailing list