RFR: 8315088: C2: assert(wq.size() - before == EMPTY_LOOP_SIZE) failed: expect the EMPTY_LOOP_SIZE nodes of this body if empty [v3]

[Tobias Hartmann]

On Mon, 4 Sep 2023 15:35:29 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> The assert fires in code that looks for counted loops that would be
>> empty if not part of a loop strip mining nest (that is some nodes in
>> the the loop body are kept alive because of the safepoint in the outer
>> loop, the loop would otherwise be empty). That logic starts by finding
>> the set of `EMPTY_LOOP_SIZE` nodes that are core to all counted loops
>> (exit test, iv phi etc) and store them in the `empty_loop_nodes`
>> list.
>> 
>> Finding the "core" nodes is performed by
>> `IdealLoopTree::collect_loop_core_nodes`. It enqueues the backedge of
>> the loop in `empty_loop_nodes` and then iteratively pushes inputs of
>> nodes in `empty_loop_nodes` that belongs to the loop until it can't
>> make progress. There should be `EMPTY_LOOP_SIZE` of those nodes.
>> 
>>  It's possible that that 2 or more loops are involved in the process:
>> a node n in loop 1 could be kept alive by the safepoint of loop 2 so
>> loop 2 needs to be empty for n to become dead and possibly loop 1 to
>> be empty. When that happens while the logic is in the process of
>> determining if loop 1 is empty, it needs to also collect the
>> `EMPTY_LOOP_SIZE` nodes that make the "core" of loop 2. They are
>> enqueued to the `empty_loop_nodes` list too.
>> 
>> The failure happens when `empty_loop_nodes` already has the "core"
>> nodes of loop 1 and "core" nodes for loop 2 are collected. In the
>> process`IdealLoopTree::collect_loop_core_nodes` iterates on all
>> `empty_loop_nodes` (loop 1 nodes included, not just starting from the
>> loop 2 backedge). That's inefficient (loop 1 nodes can't help find
>> nodes that belong to loop 2) and wrong because there could be an edge
>> from a node in loop 1 to a node in the body of loop 2. That node in
>> loop 2 is them pushed to the `empty_loop_nodes` list. 
>> 
>> The fix I propose is to only iterate over loop 2 nodes when loop 2 is
>> processed.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Marked as reviewed by thartmann (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/15520#pullrequestreview-1610399408