RFR: 8323154: C2: assert(cmp != nullptr && cmp->Opcode() == Op_Cmp(bt)) failed: no exit test
Christian Hagedorn
chagedorn at openjdk.org
Wed Jan 17 07:50:04 UTC 2024
This bug is very similar to [JDK-8314191](https://bugs.openjdk.org/browse/JDK-8314191) but with long counted loops instead of int counted loops and a with a different manifestation.
The original problem was that [JDK-8276162](https://bugs.openjdk.org/browse/JDK-8276162) added transformations for `CmpI` nodes to use `CmpU` nodes instead. The transformations were also applied for `CmpI` nodes of counted loop exit checks which messed pattern matching up. [JDK-8314191](https://bugs.openjdk.org/browse/JDK-8314191) and the follow-up fix [JDK-8316719](https://bugs.openjdk.org/browse/JDK-8316719) fixed this but only for `CountedLoopNodeEndNodes`. The newly added `is_cloop_condition()` method only checks for `is_CountedLoopEnd()` (int counted loops) instead of `is_BaseCountedLoopEnd()` (also includes long counted loop). This patch fixes this.
I've had a closer look at other uses of `is_CountedLoop*` and found that
https://github.com/openjdk/jdk/blob/1007618f6f97fad0f66e4074b50521bdd853629e/src/hotspot/share/opto/subnode.cpp#L126-L139
and
https://github.com/openjdk/jdk/blob/1007618f6f97fad0f66e4074b50521bdd853629e/src/hotspot/share/opto/subnode.cpp#L155-L163
should probably also use the `BaseCountedLoop*` versions. These methods are used in `ok_to_convert()` which is used in several places. They try to prevent transformations involving the iv and the increment node of a counted loop to save registers. However, these transformations are still applied before a loop is transformed to a counted loop. This raises the question whether these bailouts should be extended to also work before loop opts. Since this code has been around for such a long time, it would also be interesting to see, if it's still beneficial to block these optimizations in general. If so, it might be good if we could add some IR tests to prove that.
There are more places where we try to prevent such transformations but again only if we already have a counted loop. For example:
https://github.com/openjdk/jdk/blob/1007618f6f97fad0f66e4074b50521bdd853629e/src/hotspot/share/opto/addnode.cpp#L176-L191
or
https://github.com/openjdk/jdk/blob/1007618f6f97fad0f66e4074b50521bdd853629e/src/hotspot/share/opto/addnode.cpp#L193-L209
It might be a good idea to revisit all of these bailouts in general and check if it's still beneficial to have them around and if they should be extended to also work before loop opts.
I suggest to do this investigation together with fixing `CountedLoop*` -> `BaseCountedLoop*` in the methods used in `ok_to_convert()` in a separate RFE.
Thanks,
Christian
-------------
Commit messages:
- 8323154: C2: assert(cmp != nullptr && cmp->Opcode() == Op_Cmp(bt)) failed: no exit test
Changes: https://git.openjdk.org/jdk/pull/17459/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17459&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8323154
Stats: 50 lines in 2 files changed: 49 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/17459.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17459/head:pull/17459
PR: https://git.openjdk.org/jdk/pull/17459
More information about the hotspot-compiler-dev
mailing list