RFR: JDK-8287061: Support for rematerializing scalar replaced objects participating in allocation merges [v5]

Xin Liu xliu at openjdk.org
Fri Mar 31 18:33:19 UTC 2023


On Thu, 30 Mar 2023 23:36:20 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> Can I please get reviews for this PR? 
>> 
>> The most common and frequent use of NonEscaping Phis merging object allocations is for debugging information. The two graphs below show numbers for Renaissance and DaCapo benchmarks - similar results are obtained for all other applications that I tested.
>> 
>> With what frequency does each IR node type occurs as an allocation merge user? I.e., if the same node type uses a Phi N times the counter is incremented by N:
>> 
>> ![image](https://user-images.githubusercontent.com/2249648/222280517-4dcf5871-2564-4207-b49e-22aee47fa49d.png)
>> 
>> What are the most common users of allocation merges? I.e., if the same node type uses a Phi N times the counter is incremented by 1:
>> 
>> ![image](https://user-images.githubusercontent.com/2249648/222280608-ca742a4e-1622-4e69-a778-e4db6805ea02.png)
>> 
>> This PR adds support scalar replacing allocations participating in merges that are used as debug information OR as a base for field loads. I plan to create subsequent PRs to enable scalar replacement of merges used by other node types (CmpP is next on the list) subsequently.
>> 
>> The approach I used for _rematerialization_ is pretty straightforward. It consists basically in: 1) Extend SafePointScalarObjectNode to represent multiple SR objects; 2) Add a new Class to support rematerialization of SR objects part of merges; 3) Patch HotSpot to be able to serialize and deserialize debug information related to allocation merges; 4) Patch C2 to generate unique types for SR objects participating in some allocation merges.
>> 
>> The approach I used for _enabling the scalar replacement of some of the inputs of the allocation merge_ is also pretty straight forward: call `MemNode::split_through_phi` to, well, split AddP->Load* through the merge which will render the Phi useless.
>> 
>> I tested this with JTREG tests tier 1-4 (Windows, Linux, and Mac) and didn't see regression. I also tested with several applications and didn't see any failure. I also ran tests with "-ea -esa -Xbatch -Xcomp -XX:+UnlockExperimentalVMOptions -XX:-TieredCompilation -server -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:+StressLCM -XX:+StressGCM -XX:+StressCCP" and didn't observe any related failures.
>
> Cesar Soares Lucas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address PR feeedback 1: make ObjectMergeValue subclass of ObjectValue & create new IR class to represent scalarized merges.

src/hotspot/share/opto/escape.cpp line 457:

> 455:         found_sr_allocate = true;
> 456:       } else {
> 457:         ptn->set_scalar_replaceable(false);

This member function is const. Do we really need to change ptn's property here? 

My reading is ophi is profitable as long as we spot any input object which can be eliminated.  how about you just return at line 455?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12897#discussion_r1154778280



More information about the security-dev mailing list