Plan to create unique types for inputs of allocation merges

[Vladimir Ivanov]

Hi Cesar,

I assume you consider enhancing split_unique_types as part of 
JDK-8289943 [1] we discussed before.

I still think that improving split_unique_types to handle allocation 
merges is a better alternative to custom RAM nodes with an extra phase 
to optimize them.

Your proposal looks reasonable.

Speaking of Store nodes, my recollection is they aren't handled by 
pr/9073 [1] yet. (In particular, ConnectionGraph::can_reduce_this_phi() 
bails on !ConnectionGraph::is_read_only() check [2].)

I wouldn't be bothered too much by the fact that stores can't be 
reliably optimized in general case. The whole idea behind JDK-8289943 is 
to focus on "simple enough" shapes. There will always be complex enough 
code shapes the conservative analysis couldn't handle.

I'm glad you continue working on the EA enhancement. Looking forward for 
the next iteration of your work!

Best regards,
Vladimir Ivanov

[1] https://git.openjdk.org/jdk/pull/9073
[2] 
https://github.com/openjdk/jdk/pull/9073/files#diff-03f7ae3cf79ff61be6e4f0590b7809a87825b073341fdbfcf36143b99c304474R652

On 2/6/23 11:56, Cesar Soares Lucas wrote:
> Hello there!
> 
> Can I please get some feedback on the following plan?
> 
> ---
> 
> Plan to patch the split_unique_types method to make it able to create
> 
> unique types for inputs participating in NoEscape merges. The motivation for
> 
> doing this is because it will make possible to remove the allocation merge
> 
> (after making other changes) and scalar replace some of the merge's input. I
> 
> will be concurrently working on a different set of changes to eliminate the
> 
> allocation merges.
> 
> My idea is to approach this in three parts. The parts are separated by 
> the type
> 
> of node that uses the allocation merge.
> 
> 1) Merges that have only SafePoint (and subclasses) nodes as users. This 
> will
> 
> let me handle merges used as debug information in SafePoints. There are 
> not many
> 
> merges that are only used by SafePoints but SafePointNode is the most common
> 
> user of merges (the merge is used by a SafePointNode and some other type
> 
> of node).
> 
> 2) Also handle merges that have field loads as users. This will let me 
> later on
> 
> split-thru-phi the loads and after some other changes remove the merge.
> 
> With (1) and (2) in place many (maybe most) merge usage patterns can be
> 
> simplified.
> 
> 3) Also handle merges that have Store as users. This is by far the most
> 
> complicated case. It might not be possible to handle this kind of merge in
> 
> general, but we can make some cases work. My current idea for handling this
> 
> scenario is to clone the Store, make each one use a different input from the
> 
> merge and have a different unique type. This will require adding a "selector
> 
> phi" to output which input of the merge should be used and IfNode's to 
> control
> 
> the execution of the cloned Store's. I understand we'll need to limit 
> the number
> 
> of bases and stores that we can handle.
> 
> As an illustration of the 3rd scenario above, this code:
> 
> p = phi(x, o1, o2);
> 
> p.x = 10;           // no instance_id
> 
> would become:
> 
> p1 = phi(x, o1, NULL);
> 
> p2 = phi(x, NULL, o2);
> 
> sl = phi(x, 0, 1);  // selector Phi
> 
> if (sl == 0)
> 
>      p1.x = 10;      // instance_id "1"
> 
> else
> 
>      p2.x = 10;      // instance_id "2"
> 
> Best regards,
> 
> Cesar
>