RFR: 8336830: C2: assert(get_loop(lca)->_nest < n_loop->_nest || lca->in(0)->is_NeverBranch()) failed: must not be moved into inner loop

[Vladimir Kozlov]

On Thu, 25 Jul 2024 15:36:01 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> A store is sunk from a counted loop into an enclosing infinite
>> loop. The assert fires because:
>> 
>> 
>> get_loop(lca)->_nest < n_loop->_nest
>> 
>> 
>> is false. That happens because the outer loop was found to be infinite
>> in the current loop opts pass. When that happens, it's not properly
>> attached to the loop tree. The second part of the assert was added to
>> cover a similar case:
>> 
>> 
>> lca->in(0)->is_NeverBranch()
>> 
>> 
>> but it doesn't work in this case bcause lca is not a projection of the
>> `NeverBranch`. It's the exit projection of the counted loop. The fix I
>> propose changes that part of the assert to test that lca is, indeed,
>> in an infinite loop in a way that's robust.
>> 
>> I also removed some code that I believe to be useless following
>> 8335709.
>
> src/hotspot/share/opto/loopopts.cpp line 1275:
> 
>> 1273:   for (;;) {
>> 1274:     Node* dom = idom(useblock);
>> 1275:     if (loop->is_member(get_loop(dom))) {
> 
> This would not be backported, right? Do you think we should do it in a separate RFE? Or is it necessary for the fix?

Yes, it should be separate RFE  or you back port [JDK-8335709](https://bugs.openjdk.org/browse/JDK-8335709) first.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20334#discussion_r1697284939