RFR: 8289943: Simplify some object allocation merges [v8]

[Vladimir Kozlov]

On Tue, 20 Sep 2022 18:20:15 GMT, Cesar Soares <duke at openjdk.org> wrote:

>> Hi there, can I please get some feedback on this approach to simplify object allocation merges in order to promote Scalar Replacement of the objects involved in the merge?
>> 
>> The basic idea for this [approach was discussed in this thread](https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2022-April/055189.html) and it consists of: 
>> 1) Identify Phi nodes that merge object allocations and replace them with a new IR node called ReducedAllocationMergeNode (RAM node).
>> 2) Scalar Replace the incoming allocations to the RAM node.
>> 3) Scalar Replace the RAM node itself.
>> 
>> There are a few conditions for doing the replacement of the Phi by a RAM node though - Although I plan to work on removing them in subsequent PRs:
>> 
>> - The only supported users of the original Phi are AddP->Load, SafePoints/Traps, DecodeN.
>> 
>> These are the critical parts of the implementation and I'd appreciate it very much if you could tell me if what I implemented isn't violating any C2 IR constraints:
>> 
>> - The way I identify/use the memory edges that will be used to find the last stored values to the merged object fields.
>> - The way I check if there is an incoming Allocate node to the original Phi node.
>> - The way I check if there is no store to the merged objects after they are merged.
>> 
>> Testing:
>> - Windows/Linux/MAC fastdebug/release 
>>   - hotspot_all
>>   - tier1
>>   - Renaissance
>>   - dacapo
>>   - new IR-based tests
>
> Cesar Soares has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove debug messages.

Please, merge the latest JDK sources after you address my comments. I can submit testing after that.
And you will need second review.

src/hotspot/share/opto/callnode.cpp line 1834:

> 1832:     for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
> 1833:       Node* addp = n->fast_out(i);
> 1834:       assert(addp->is_AddP(), "DecodeN user is not an AddP");

You don't need this assert since `as_AddP()` has it.

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

> 591:     }
> 592: 
> 593:     klass = input_t->is_instptr()->instance_klass();

Why you are updating `Klass` value here?

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

> 664:           return false;
> 665:         }
> 666: 

You are missing check for constant offset as you did in previous code. May be move this code into small method.
Also this whole code which checks users could be separate method.

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

> 681:   }
> 682: 
> 683:   if (mixed_klasses && has_call_as_user) {

Add comment that only the same Klass is allowed if merged allocations are referenced by SafePoint or Uncommon
trap.

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

PR: https://git.openjdk.org/jdk/pull/9073