RFR: 8326638: Crash in PhaseIdealLoop::remix_address_expressions due to unexpected Region instead of Loop

[Christian Hagedorn]

On Mon, 26 Feb 2024 12:42:53 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> src/hotspot/share/opto/loopopts.cpp line 586:
>> 
>>> 584:           Node* add1 = new AddPNode(n->in(1), n->in(2)->in(2), n->in(3));
>>> 585:           // Stuff new AddP in the loop preheader
>>> 586:           Node* entry = n_loop->_head->is_Loop() ? n_loop->_head->as_Loop()->skip_strip_mined(1)->in(LoopNode::EntryControl)
>> 
>> Should we add a sanity assert (which could also serve as an explaining comment) that in the `RegionNode` case, it must be an irreducible loop head?
>
> I could do that but it would feel a bit arbitrary to put the assert only in this code, given that we have the same `is_Loop` check in a lot of other code as well. Maybe it would make sense to factor the checks into a separate method at some point.

Makes sense to do a complete replacement of this pattern in other code as well at some point. But let's do that separately.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18002#discussion_r1502575329