RFR: 8348572: C2 compilation asserts due to unexpected irreducible loop [v2]

[Emanuel Peter]

On Thu, 30 Jan 2025 17:24:07 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   update for Vladimir
>
> Few comments.

@vnkozlov Thanks for reviewing! I think I addressed all your comments / questions 😊

> src/hotspot/share/opto/c2_globals.hpp line 636:
> 
>> 634:           "verify major loop optimizations")                                \
>> 635:                                                                             \
>> 636:   develop(bool, VerifyNoNewIrreducibleLoops, false,                         \
> 
> Do you plan to add it to our stress testing?

We cannot currently do that:

> With the new debug flag -XX:+VerifyNoNewIrreducibleLoops, we still hit the assert, as expected:

And we can do this later:

> For now, have the assert behind a Verify flag, so that [JDK-8348570](https://bugs.openjdk.org/browse/JDK-8348570) is unblocked. Later, we can remove the Verify flag and alway enable the assert again.

Does that sound ok to you?

>Do you mean "And it is not a Loop node" (based on condition code below)?

I changed it to an "And" now.

>Do we have an assert which checks that LoopNode is never marked MaybeIrreducibleEntry?

I added an assert for `set_loop_status`, just in case.

> src/hotspot/share/opto/loopnode.cpp line 5635:
> 
>> 5633:       RegionNode* secondary_entry = m->as_Region();
>> 5634: 
>> 5635:       if (!secondary_entry->can_be_irreducible_entry()) {
> 
> Typo? Why negation?

If a secondary entry can not be a secondary (irreducible) entry, then we have a problem, so we do a bailout.

Does that make sense?

> src/hotspot/share/opto/loopnode.cpp line 5646:
> 
>> 5644:         l->_irreducible = 1; // = true
>> 5645:         RegionNode* head = l->_head->as_Region();
>> 5646:         if (!head->can_be_irreducible_entry()) {
> 
> Same issue here?

Same explanation as above :)

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

PR Comment: https://git.openjdk.org/jdk/pull/23363#issuecomment-2626372853
PR Review Comment: https://git.openjdk.org/jdk/pull/23363#discussion_r1936717575
PR Review Comment: https://git.openjdk.org/jdk/pull/23363#discussion_r1936715840
PR Review Comment: https://git.openjdk.org/jdk/pull/23363#discussion_r1936716563
PR Review Comment: https://git.openjdk.org/jdk/pull/23363#discussion_r1936717908