RFR: 8297724: Loop strip mining prevents some empty loops from being eliminated

[Tobias Hartmann]

On Thu, 15 Dec 2022 16:43:07 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> When an empty loop is found, it's removed and as a consequence the
> outer strip mine loop and the safepoint that it contains are also
> removed. A counted loop is empty if it has the minimum number of nodes
> that a well formed counted loop contains. In some cases, the loop has
> extra nodes and the safepoint in the outer loop is the only node that
> keeps those extra nodes alive. If the safepoint was to be removed,
> then the counted loop would have the minimum number of nodes and be
> considered empty. But the safepoint can't be removed until the loop is
> considered empty which only happens if it has the minimum of nodes. As
> a result, these loops are not removed. Note that now that the loop
> strip mining loop nest is constructed even if UseCountedLoopSafepoints
> is false, there's a regression where some loops used to be removed as
> empty before but not anymore.
> 
> The fix I propose is to extend IdealLoopTree::do_remove_empty_loop()
> so it handles those cases. If it encounters a loop with no flow
> control in the loop body but a number of nodes greater than the
> minimum number of nodes, it starts from the extra nodes in the loop
> body and follows uses until it finds a side effect, ignoring the
> safepoint of the outer loop. If it finds none, then the extra nodes
> can be removed and the loop is empty. This also works if the extra
> nodes are kept alive by the safepoints of 2 different counted loops
> and one can only be proven empty if the other one is as well (and the
> other one proven empty if the first one is) and should work even if
> there are more than 2 nodes involved..

Looks good to me. I'm running some testing and will report back once it passed.

src/hotspot/share/opto/loopTransform.cpp line 3598:

> 3596:   CountedLoopNode *cl = _head->as_CountedLoop();
> 3597: #ifdef ASSERT
> 3598:   // Call collect_loop_core_nodes to exercise the assert that check that it finds the right number of nodes

Suggestion:

  // Call collect_loop_core_nodes to exercise the assert that checks that it finds the right number of nodes

test/hotspot/jtreg/compiler/c2/irTests/TestLSMMissedEmptyLoop.java line 30:

> 28: /*
> 29:  * @test
> 30:  * @library /test/lib /

Suggestion:

 * @test
 * @bug 8297724
 * @library /test/lib /

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

Marked as reviewed by thartmann (Reviewer).

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