RFR: 8342330: C2: "node pinned on loop exit test?" assert failure [v2]

[Roland Westrelin]

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

Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:

  Update src/hotspot/share/opto/loopopts.cpp
  
  Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/21601/files
  - new: https://git.openjdk.org/jdk/pull/21601/files/2fd46262..cbd2ed62

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=21601&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21601&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/21601.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/21601/head:pull/21601

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