RFR: 8341293: Split field loads through Nested Phis [v9]

[Dhamoder Nalla]

On Tue, 8 Apr 2025 20:21:34 GMT, Dhamoder Nalla <dhanalla at openjdk.org> wrote:

>> This enhances the changes introduced in [JDK PR 12897](https://github.com/openjdk/jdk/pull/12897) by handling nested Phi nodes (phi -> phi -> AddP -> Load*) during scalar replacement. The primary goal is to split field loads (AddP -> Load*) involving nested Phi parent nodes, thereby increasing opportunities for scalar replacement and reducing memory allocations.
>> 
>> 
>> **Here is an illustration of the sequence of Ideal Graph Transformations applied to split through nested `Phi` nodes.**
>>  
>> **1. Initial State (Before Transformation)**
>> The graph contains a nested Phi structure where two Allocate nodes merge via a Phi node.
>> 
>> ![image](https://github.com/user-attachments/assets/c18e5ca0-c554-475c-814a-7cb288d96569)
>> 
>> **2. After Splitting Through Child Phi**
>> The transformation separates field loads by introducing additional AddP and Load nodes for each Allocate input.
>> 
>> ![image](https://github.com/user-attachments/assets/b279b5f2-9ec6-4d9b-a627-506451f1cf81)
>> 
>> **3. After Splitting Load Field Through Parent Phi**
>> The field load operation (Load) is pushed even further up in the graph.
>> 
>> Instead of merging AddP pointers in a Phi node and then performing a Load, the transformation ensures that each path has its AddP -> Load sequence before merging.
>> 
>> This further eliminates the need to perform field loads on a Phi node, making the graph more conducive to scalar replacement.
>> 
>> ![image](https://github.com/user-attachments/assets/f506b918-2dd0-4dbe-a440-ff253afa3961)
>> 
>> ### JMH Benchmark Results:
>> 
>> #### With Disabled RAM
>> 
>> | Benchmark | Mode | Count | Score | Error | Units |
>> |-----------|------|-------|-------|-------|-------|
>> | testBailOut_runner | avgt | 15 | 13.969 | ± 0.248 | ms/op |
>> | testFieldEscapeWithMerge_runner | avgt | 15 | 80.300 | ± 4.306 | ms/op |
>> | testMerge_TryCatchFinally_runner | avgt | 15 | 72.182 | ± 1.781 | ms/op |
>> | testMultiParentPhi_runner | avgt | 15 | 2.983 | ± 0.001 | ms/op |
>> | testNestedPhiPolymorphic_runner | avgt | 15 | 18.342 | ± 0.731 | ms/op |
>> | testNestedPhiProcessOrder_runner | avgt | 15 | 14.315 | ± 0.443 | ms/op |
>> | testNestedPhiWithLambda_runner | avgt | 15 | 18.511 | ± 1.212 | ms/op |
>> | testNestedPhiWithTrap_runner | avgt | 15 | 66.277 | ± 1.478 | ms/op |
>> | testNestedPhi_FieldLoad_runner | avgt | 15 | 17.968 | ± 0.306 | ms/op |
>> | testNestedPhi_TryCatch_runner | avgt | 15 | 14.186 | ± 0.247 | ms/op |
>> | testRematerialize_MultiObj_runner | avgt | 15 | 88.435 | ± 4.869 ...
>
> Dhamoder Nalla has updated the pull request incrementally with one additional commit since the last revision:
> 
>   address CR comments

> Looks like you deleted some of the tests you had there. Can you explain why? This was not by any chance the one that failed? [3c56f98](https://github.com/openjdk/jdk/commit/3c56f98d88433a4fada2c7e43147fc2e91df5e89)

Thanks @eme64 for taking a look at it,
I haven't deleted any tests; test cases list below is unchanged.

  try {
            Asserts.assertEQ(testRematerialize_SingleObj_Interp(cond1, x, y),       testRematerialize_SingleObj_C2(cond1, x, y));
        } catch (Exception e) {}
        Asserts.assertEQ(testRematerialize_TryCatch_Interp(cond1, l, x, y),         testRematerialize_TryCatch_C2(cond1, l, x, y));
        Asserts.assertEQ(testMerge_TryCatchFinally_Interp(cond1, l, x, y),          testMerge_TryCatchFinally_C2(cond1, l, x, y));
        Asserts.assertEQ(testRematerialize_MultiObj_Interp(cond1, cond2, x, y),     testRematerialize_MultiObj_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testGlobalEscapeInThread_Intrep(cond1, l, x, y),           testGlobalEscapeInThread_C2(cond1, l, x, y));
        Asserts.assertEQ(testGlobalEscapeInThreadWithSync_Intrep(cond1, x, y),      testGlobalEscapeInThreadWithSync_C2(cond1, x, y));
        Asserts.assertEQ(testFieldEscapeWithMerge_Intrep(cond1, x, y),              testFieldEscapeWithMerge_C2(cond1, x, y));
        Asserts.assertEQ(testNestedPhi_FieldLoad_Interp(cond1, cond2, x, y),        testNestedPhi_FieldLoad_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testThreeLevelNestedPhi_Interp(cond1, cond2, x, y),        testThreeLevelNestedPhi_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testNestedPhiProcessOrder_Interp(cond1, cond2, x, y),      testNestedPhiProcessOrder_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testNestedPhi_TryCatch_Interp(cond1, cond2, x, y),         testNestedPhi_TryCatch_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testBailOut_Interp(cond1, cond2, x, y),                    testBailOut_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testNestedPhiPolymorphic_Interp(cond1, cond2, x, y),       testNestedPhiPolymorphic_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testNestedPhiWithTrap_Interp(cond1, cond2, x, y),          testNestedPhiWithTrap_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testNestedPhiWithLambda_Interp(cond1, cond2, x, y),        testNestedPhiWithLambda_C2(cond1, cond2, x, y));
        Asserts.assertEQ(testMultiParentPhi_Interp(cond1, x, y),                    testMultiParentPhi_C2(cond1, x, y));

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

PR Comment: https://git.openjdk.org/jdk/pull/21270#issuecomment-2830685272