Integrated: 8323154: C2: assert(cmp != nullptr && cmp->Opcode() == Op_Cmp(bt)) failed: no exit test

[Christian Hagedorn]

On Wed, 17 Jan 2024 07:44:28 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

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

This pull request has now been integrated.

Changeset: 6997bfc6
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/6997bfc68def7f80fbf6a7486a4b9f61225fc471
Stats:     50 lines in 2 files changed: 49 ins; 0 del; 1 mod

8323154: C2: assert(cmp != nullptr && cmp->Opcode() == Op_Cmp(bt)) failed: no exit test

Reviewed-by: roland, thartmann, qamai

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

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