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

Tobias Hartmann thartmann at openjdk.org
Thu Jun 13 05:55:13 UTC 2024


On Tue, 11 Jun 2024 02:23:36 GMT, MaxXing <duke 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.
>
> MaxXing has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add IR test and update copyright.

I executed some quick testing and I see a failure:


One or more @IR rules failed:

Failed IR Rules (1) of Methods (1)
----------------------------------
1) Method "private int compiler.c2.irTests.scalarReplacement.ScalarReplacementWithGCBarrierTests.testScalarReplacementWithGCBarrier(compiler.c2.irTests.scalarReplacement.ScalarReplacementWithGCBarrierTests$List)" - [Failed IR rules: 1]:
   * @IR rule 2: "@compiler.lib.ir_framework.IR(phase={INCREMENTAL_BOXING_INLINE}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#ALLOC#_", "2"}, failOn={}, applyIfPlatform={}, applyIfPlatformOr={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
     > Phase "Incremental Boxing Inline":
       - counts: Graph contains wrong number of nodes:
         * Constraint 1: "(\\d+(\\s){2}(Allocate\\b.*)+(\\s){2}===.*)"
           - Failed comparison: [found] 1 = 2 [given]
             - Matched node:
               * 62  Allocate  === 30 6 7 8 1 (60 59 41 1 1 1 38 1 38 1 ) [[ 63 64 65 72 73 74 ]]  rawptr:NotNull ( int:>=0, java/lang/Object:NotNull *, bool, top, bool ) ScalarReplacementWithGCBarrierTests$List::iter @ bci:0 (line 48) ScalarReplacementWithGCBarrierTests::testScalarReplacementWithGCBarrier @ bci:1 (line 106) !jvms: ScalarReplacementWithGCBarrierTests$List::iter @ bci:0 (line 48) ScalarReplacementWithGCBarrierTests::testScalarReplacementWithGCBarrier @ bci:1 (line 106)

Here's the full log file: [log.txt](https://github.com/user-attachments/files/15814400/log.txt)

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

PR Comment: https://git.openjdk.org/jdk/pull/19496#issuecomment-2164479914


More information about the hotspot-compiler-dev mailing list