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

[Emanuel Peter]

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?
https://github.com/openjdk/jdk/pull/21270/commits/3c56f98d88433a4fada2c7e43147fc2e91df5e89

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

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