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

Emanuel Peter epeter at openjdk.org
Wed Feb 22 14:58:37 UTC 2023


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`.

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

Commit messages:
 - 8303045: Remove RegionNode::LoopStatus::NeverIrreducibleEntry assert with wrong assumption

Changes: https://git.openjdk.org/jdk/pull/12713/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12713&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8303045
  Stats: 114 lines in 3 files changed: 113 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/12713.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12713/head:pull/12713

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


More information about the hotspot-compiler-dev mailing list