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

[Jorn Vernee]

On Wed, 27 Oct 2021 10:27:38 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixup all platform stubs
>
> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/CallingSequenceBuilder.java line 91:
> 
>> 89:     }
>> 90: 
>> 91:     private boolean isImr() {
> 
> I have to say I'm a bit confused by this IMR term. As I'm sure you are aware, we use this term also in the call arrangers, to denote real ABI in-memory-return (e.g. when a struct is too big to be returned in register). The meaning of IMR here is different - and it essentially means: returned in more than one register. I don't think we will be able to merge the two IMR concepts, given one is an ABI concept (e.g. the ABI requires an extra pointer to be passed) and the other is a CLinker specific concept (e.g. an extra segment we pass to store multi-register returns). Because of that, I think we probably need a different name here, to avoid confusion (e.g. `needsReturnBuffer` etc.)

I agree a better name is needed, but got stuck trying to think of one. I think `needsReturnBuffer` is an excellent suggestion though. I'll go ahead and apply that.

> 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.

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

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