[External] : Re: API to create a new Allocate node?

[Vladimir Kozlov]

On 4/29/22 3:41 PM, John Rose wrote:
> Vladimir: An EA-suppressed allocation has its own debug info, and the deopt handler uses that info to weave together 
> heap-allocated versions of the EA-suppressed allocations, so the interpreter has something normal to work on. Isn’t that 
> what would naturally appear at the call to |foo()|?

We currently handle only the case with one allocation and use its `instance_id` (node's id) to search for its fields values:
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/macro.cpp#L343

Phi of allocations does not have such ID. And currently we don't have corresponding allocations field's Phis. So we 
can't construct debug info for deoptimization after merge.

I am trying to lead Cesar to a solution which would help this merge case. As he suggested we may need some kind of 
virtual object node which references allocations and their fields through Phis and has own ID which we can use to 
construct debug info for deoptimization. And it will be gone after macro expansion - replaced with 
SafePointScalarObjectNode or simply removed if it is not referenced.

We have to put some restrictions/checks on such "virtual object" to make sure it is handled correctly during IR 
transformations.

Vladimir K

> 
> Cesar: If the AllocateNode is not going to actually code-gen and execute, then there is less harm in creating it, but I 
> like Vladimir’s suggestion much better, that you use a special kind of node, not a subclass of SafePointNode. The 
> problem is that you /could/ insert an AllocateNode which “tells lies” about its JVMS, and you /could/ promise yourself 
> that this is harmless because it will never get code generated, but there are at least two ways the promise could be 
> violated: First, a bug might lead for the node to be generated instead of removed. Second, some other pass in the IR 
> might try to copy and repurpose the fake JVMS stored on the inserted AllocateNode: IIRC there /are such algorithms/ in 
> C2 which hunt around in the IR for JVMS information.
> 
> In general, IR routinely provides demonstrations of the old adage, “Oh what a tangled web we weave when first we 
> practice to deceive.”
> 
> On 29 Apr 2022, at 15:06, Vladimir Kozlov wrote:
> 
>     In such case you may not need Allocation node. You can try a simple new ideal node "ScalarObjectNode" which track
>     allocation and its fields if there is guarantee that no safepoints in between. Something like
>     SafePointScalarObjectNode but without referencing debug info (JVM state).
> 
>     But I think you also need to consider more complex case: if there is Safepoint or Call between merge point and
>     allocation reference after it:
> 
>     if (...) {
>     p0 = new Point();
>     } else {
>     p1 = new Point();
>     }
>     foo(); // Allocation does not escape but it is refernced in debug info in JVMS of this call.
>     p = phi(p0, p1);
>     return p.x;
> 
>     I am not sure we currently would handle such case when scalarizing allocations in macro.cpp.
> 
>     Thanks,
>     Vladimir K
> 
>     On 4/29/22 1:44 PM, Cesar Soares Lucas wrote:
> 
>         Thank you for taking the time to write a detailed explanation, John. That helps a lot!
> 
>         One thing that perhaps I left implicit in my original comment: this new allocation shouldn't be part of the
>         final code emitted by C2. The allocation will be used just to remove the phi node merging other objects and
>         therefore EA+SR should be able to remove all allocations involved. Please, take a look at the example below.
> 
>         C2 can't remove the allocations in the code below because of the Phi merging the objects:
> 
>         if (...)
>         p0 = new Point();
>         else
>         p1 = new Point();
>         p = phi(p0, p1);
>         return p.x;
> 
>         What I'm proposing is to replace the Phi *temporarily* with an allocation like shown below. C2 should be able to
>         scalar replace all three objects in the example.
> 
>         if (...)
>         p0 = new Point(0);
>         else
>         p1 = new Point(1);
>         p = new Point(phi(p0.x, p1.x));
>         return p.x;
> 
>         In the final code we shouldn't have any allocation (and so no need to pick up a JVMS?!):
> 
>         if (...)
>         x0 = 0;
>         else
>         x1 = 1;
>         px = phi(x0, x1);
>         return px;
> 
>         Best regards,
> 
>         Cesar
>