[foreign-memaccess+abi] RFR: 8255903: Enable multi-register return values for native invokers [v3]

[Maurizio Cimadamore]

On Thu, 28 Oct 2021 13:28:15 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/ProgrammableInvoker.java line 277:
>> 
>>> 275:                 : Binding.Context.DUMMY;
>>> 276:         try (unboxContext) {
>>> 277:             MemorySegment imrSegment = null;
>> 
>> The logic in this method seems fine. I have a question: since IMR is only used when we need to return a segment, and since in these cases the client has to provide a SegmentAllocator, why can't we use the client allocator to create the IMR (and then tell the VM where to store the register values into the user-allocated segment) ? The code seems to allocate a temp segment on the linker scope, copy the multi-reg return in there, then, copy again the data from the temp segment back to the user segment. Which seems odd given that the user is already giving us a segment where to store the results?
>
> In theory merging the two would be possible, but it would significantly blur the line between which parts of the binding recipe are handled by Java code, and which parts are handled by the VM code: currently the VM only handles the move bindings, but it would have to be re-written to handle buffer stores in certain cases as well. As you say, the problem is that the VM doesn't know at which offsets the return values should be stored.
> 
> I thought of other solutions here as well, like using a Java object instead of an off-heap buffer to hold the intermediate values which would make the allocation cheaper, or to wait for Valhalla, which would give us Java types that take up multiple registers when returning as well, in which case those could be used as a carrier type instead of having an intermediate return buffer. (the latter seems like the most attractive option to me).
> 
> So, I punted on this problem for now. My main objective of adding support for multi-register returns with this patch is to be able to remove the buffered invocation strategy.

I was thinking the same, and I thought that using an intermediate object (not a segment) would probably be a good idea. E.g.

class ReturnBuffer {
   long first;
   long second;
}

For now this is just a plain identity class. When we have Valhalla, we can make this a value. But I think even now it should work more efficiently than allocating a segment with malloc (as I hope this object will be scalarized).

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

PR: https://git.openjdk.java.net/panama-foreign/pull/603