On Mon, 17 Apr 2023 16:17:30 GMT, Cesar Soares Lucas <cslu...@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 
>> 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 of the following. 1) New IR node (suggested by V. 
>> Kozlov), named SafePointScalarMergeNode, to represent a set of 
>> SafePointScalarObjectNode; 2) Each scalar replaceable input participating in 
>> a merge will get a SafePointScalarObjectNode like if it weren't part of a 
>> merge. 3) Add a new Class to support the rematerialization of SR objects 
>> that are part of a merge; 4) Patch HotSpot to be able to serialize and 
>> deserialize debug information related to allocation merges; 5) 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 straightforward: 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 experimented 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:
> 
>   Fix tests. Remember previous reducible Phis.

Also next 2 JVMCI tests failed:

compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaMethod.java
compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java

#  Internal Error 
(/workspace/open/src/hotspot/cpu/x86/macroAssembler_x86.cpp:829), pid=2430194, 
tid=2430218
#  fatal error: DEBUG MESSAGE: exact klass and actual klass differ

Could be due to [12810](https://git.openjdk.org/jdk/pull/12810)

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

PR Comment: https://git.openjdk.org/jdk/pull/12897#issuecomment-1515556571

Reply via email to