Integrated: 8342330: C2: "node pinned on loop exit test?" assert failure

[Roland Westrelin]

On Mon, 21 Oct 2024 08:40:31 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The assert fires because range check elimination processes a test of
> the shape:
> 
> 
> if (i * 4 != (x - objectField.intField) - 1)) {
>   ...
> }
> 
> 
> and `(x - objectField.intField) - 1)` has control on the exit
> projection of the pre loop.
> 
> This happens because:
> 
> - `objectField.intField` depends on the null check of `objectField`
>   which is performed in the pre loop.
>   
> - `i * scale + (objectField.intField + 1) == x` is transformed into:
>   `i * scale == x - (objectField.intField + 1)`
> 
> - `(x - objectField.intField) - 1)` only has uses out of the pre loop
>   and is sunk out of the loop. It ends up pinned on the the exit
>   projection of the pre loop.
>   
> 
> There is already logic in `PhaseIdealLoop::ctrl_of_use_out_of_loop()`
> to handle similar cases but, here, the difference is that the use
> (`SubI` of 1) for what's being sunk doesn't have control in the main
> loop but between the pre and main loop so that logic doesn't catch
> this case.
> 
> There is also a possible bug in that logic:
> 
> 
> n_loop->_next == get_loop(u_loop->_head->as_CountedLoop()->skip_strip_mined())
> 
> 
> assumes the loop that follows the pre loop in the loop tree is the
> main loop which is not guaranteed.
> 
> In this particular case, the assert is harmless: RCE can't eliminate
> the condition but it's hard to rule out a similar scenario with a
> condition that RCE could remove. I propose revisiting the condition in
> `PhaseIdealLoop::ctrl_of_use_out_of_loop()` so it skips all uses that
> are dominated by the loop exit of the pre loop.

This pull request has now been integrated.

Changeset: 004aaea7
Author:    Roland Westrelin <roland at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/004aaea76db091569aa88eeb6b08db3408f288cd
Stats:     86 lines in 2 files changed: 82 ins; 0 del; 4 mod

8342330: C2: "node pinned on loop exit test?" assert failure

Reviewed-by: chagedorn, thartmann

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

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