RFR: 8333334: C2: Make result of `Node::dominates` more precise to enhance scalar replacement [v4]

[Tobias Hartmann]

On Tue, 25 Jun 2024 07:32:29 GMT, Qizheng Xing <qxing at openjdk.org> wrote:

>> This patch changes the algorithm of `Node::dominates` to make the result more precise, and allows the iterators of `ConcurrentHashMap` to be scalar replaced.
>> 
>> The previous algorithm will return a conservative result when encountering a dead control flow, and only try the first two input paths of a multi-input Region node, which may prevent the scalar replacement in some cases.
>> 
>> For example, with G1 GC enabled, C2 generates GC barriers for `ConcurrentHashMap` iteration operations at some early phases, and then eliminates them in a later IGVN, but `LoadNode` is also idealized in the same IGVN. This causes `LoadNode::Ideal` to see some dead barrier control flows, and refuse to split some instance field loads through Phi due to the conservative result of `Node::dominates`, and thus the scalar replacement can not be applied to iterators in the later macro elimination phase.
>> 
>> This patch allows `Node::dominates` to try other paths of the last multi-input Region node when the first path is dead, and makes `ConcurrentHashMap` iteration ~30% faster:
>> 
>> 
>> Benchmark                            (nkeys)  Mode  Cnt        Score       Error  Units
>> Maps.testConcurrentHashMapIterators    10000  avgt   15   414099.085 ± 33230.945  ns/op   # baseline
>> Maps.testConcurrentHashMapIterators    10000  avgt   15   315490.281 ±  3037.056  ns/op   # patch
>> 
>> 
>> Testing: tier1-4.
>
> Qizheng Xing has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Add `@ForceInline` to the IR test.
>  - Add IR test and update copyright.
>  - Revert last commit, and push the `LoadNode` back to the worklist to wait for the dead code to be removed.
>  - Make `Node::dominates` more precise so that iterators of `ConcurrentHashMap` can be scalar replaced.

test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java line 85:

> 83:         // Must use G1 GC to ensure there is a pre-barrier
> 84:         // before the first field write.
> 85:         TestFramework.runWithFlags("-XX:+UseG1GC");

This will fail if a different GC is selected via the command line. Your test needs a corresponding `@requires`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19496#discussion_r1660546617