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

[Cesar Soares Lucas]

On Fri, 24 Mar 2023 19:02:57 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Cesar Soares Lucas has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add support for SR'ing some inputs of merges used for field loads
>
> src/hotspot/share/code/debugInfo.hpp line 199:
> 
>> 197: // ObjectValue describing an object that was scalar replaced.
>> 198: 
>> 199: class ObjectMergeValue: public ScopeValue {
> 
> Why you did not make subclass of ObjectValue? You would need to check `sv->is_object_merge()` first before `sv->is_object()` in few places. But on other hand you don't need to duplicates ObjectValue`s fields and asserts.

Let me try that and see how it looks.

> src/hotspot/share/opto/callnode.hpp line 511:
> 
>> 509: // by a SafePoint; 2) A scalar replaced object is participating in an allocation
>> 510: // merge (Phi) and the Phi is referenced by a SafePoint. The schematics of how
>> 511: // 'spobj' is used in both scenarios are described below.
> 
> I am not comfortable with reusing SafePointScalarObjectNode for 2) since it describes totally different information.
> I think it should be separate Node which points to array of  SFSO  id (in addition to Phis) similar how we do now if SFSO is referenced in other SFSO's field.   SFSO could be created before the merge. Consider:
> 
>   Point p = new Point();
>   Point q = foo();
>   if (cond) {
>       q = p;
>   }
>   trap(p, q);

I had considered that but decided not to do it to prevent adding a new IR node. I'll give that a shot and update this thread with how it goes.

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

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