RFR: 8260653: Unreachable nodes keep speculative types alive [v2]

[Vladimir Ivanov]

On Wed, 17 Feb 2021 14:35:59 GMT, Nils Eliasson <neliasso at openjdk.org> wrote:

>> The RunThese test fails because after first igvn.optimize() (directly after parsing) there are unreachable nodes cycles left. Later when remove_speculative_types() is called - only reachable nodes will have their speculative type removed. At the end of remove_speculative_types() there is an assert that all speculative types have been removed that will fail.
>> 
>> This problem will not cause crashes in production - it is only a sanity test.
>> 
>> I suggest adding a call to PhaseRemoveUseless before remove_speculative_types. This will cost a few extra cycles but it is the only way we can guarantee that no unreachable nodes are left.
>> 
>> When debugging this I experimented with adding a call to verify_graph_edges to check for dead code at the same spot. This triggers failures in a lot of test. The conclusion is that it is very common that we have dead node cycles - but they very rarely keep speculative types alive.
>> 
>> A big thank you to Dean Long how created the reproducer for this bug.
>> 
>> Please review.
>
> Nils Eliasson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Check if node is live before assert

Looks good.

src/hotspot/share/opto/phaseX.cpp line 341:

> 339:        n != sentinel_node &&
> 340:        n->is_Type() &&
> 341:        n->outcnt() > 0 &&

`live_nodes.member(n)` makes ` n->outcnt() > 0` redundant, doesn't it?

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

Marked as reviewed by vlivanov (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2606