RFR: 8290781: Segfault at PhaseIdealLoop::clone_loop_handle_data_uses

[Christian Hagedorn]

On Wed, 24 Aug 2022 07:58:11 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> In the test case, both:
> 
>             v1 = array[i];
> (in the first loop), and
> 
>             v += array[i-1];
> 
> (in the second one) access the same element. The bound check for the
> second access is optimized out and the load of that access becomes
> control dependent on the range check of the first one.
> 
> In the context of loop strip mining, data nodes that are in the outer
> loop are expected to be reachable from the safepoint node. There are
> rare case when it's not the case so I added logic to fix those cases
> before loop cloning. That logic covers both nodes that have control in
> the outer loop and control input in the outer loop. That logic is
> incorrect (in the case of a node with a control input in the outer
> loop): when cloning the first loop body, that logic finds the load
> referenced from the range check in the loop body. That load has a
> control input that's in the inner loop. But the logic only check
> whether it's in the outer loop: anything in the inner loop is also in
> the outer loop with that logic. The control of the load is then
> wrongly updated to be outside the outer loop. That then causes a crash
> because the load is recorded as being in the body of the second loop
> but its control is not.
> 
> The fix I propose stengthen the logic that checks for nodes in the
> outer loop: it checks that the control input is in the outer loop but
> not in the inner loop.

Looks good to me!

src/hotspot/share/opto/loopopts.cpp line 2168:

> 2166:       // nodes pinned with control in the outer loop but not referenced from the safepoint must be moved out of
> 2167:       // the outer loop too
> 2168:         Node *u_c = u->in(0);

Suggestion:

        Node* u_c = u->in(0);

src/hotspot/share/opto/loopopts.cpp line 2170:

> 2168:         Node *u_c = u->in(0);
> 2169:         if (u_c != NULL) {
> 2170:           IdealLoopTree *u_c_loop = phase->get_loop(u_c);

Suggestion:

          IdealLoopTree* u_c_loop = phase->get_loop(u_c);

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

Marked as reviewed by chagedorn (Reviewer).

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