RFR: JDK-8287061: Support for rematerializing scalar replaced objects participating in allocation merges [v12]
Vladimir Ivanov
vlivanov at openjdk.org
Mon May 8 18:02:33 UTC 2023
On Mon, 1 May 2023 20:20:51 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:
>>
>> 
>>
>> 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:
>>
>> 
>>
>> 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 with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
>
> - Merge remote-tracking branch 'origin/master' into rematerialization-of-merges
> - Address part of PR review 4 & fix a bug setting only_candidate
> - Catching up with master
>
> Merge remote-tracking branch 'origin/master' into rematerialization-of-merges
> - Fix tests. Remember previous reducible Phis.
> - Address PR review 3. Some comments and be able to abort compilation.
> - Merge with Master
> - Addressing PR review 2: refactor & reuse MacroExpand::scalar_replacement method.
> - Address PR feeedback 1: make ObjectMergeValue subclass of ObjectValue & create new IR class to represent scalarized merges.
> - Add support for SR'ing some inputs of merges used for field loads
> - Fix some typos and do some small refactorings.
> - ... and 2 more: https://git.openjdk.org/jdk/compare/561ec9c5...542c5ef1
It took longer than I expected, but I finished looking into debug info.
A couple of minor comments first:
* Please, ensure that the AllocationMergesTests.java has cases to trigger the case when SRs and NSRs meet at a merge point. I was not able to provoke it with the unit test.
* diagnostic output becomes much harder to read (sample output follows).
Sample output:
- ordniary SR case
Expression stack
- @0: obj: ID=1335, only_merge_candidate=0, skip_field_assignment=0, N.Fields=4, klass: java.lang.String
Fields: 0, 0, 0, nullptr
...
Objects
obj: ID=1335, only_merge_candidate=0, skip_field_assignment=0, N.Fields=4, klass: java.lang.String
Fields: 0, 0, 0, nullptr
- mixed merge case:
ScopeDesc(pc=0x00000001080bc664 offset=1824):
java.lang.String::substring at 8 (line 2830)
Locals
- l0: merge: ID=1781, N.Candidates=1
...
Objects
merge: ID=1781, N.Candidates=1obj: ID=1782, only_merge_candidate=1, skip_field_assignment=0, N.Fields=4, klass: java.lang.String
Fields: 0, 0, 0, reg rfp [58],oop
-------------
PR Comment: https://git.openjdk.org/jdk/pull/12897#issuecomment-1538801137
More information about the hotspot-dev
mailing list