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

[Jorn Vernee]

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

>> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/FunctionDescriptor.java line 126:
>> 
>>> 124:      * @return the new function descriptor.
>>> 125:      */
>>> 126:     public FunctionDescriptor withAppendedArgumentLayouts(MemoryLayout... addedLayouts) {
>> 
>> I think these should just be called appendArgumentLayouts/insertArgumentLayouts - or, appendArguments/insertArguments - no need for all this verbosity (but not related to your patch).
>
> This method should be overridden in the variadic function case and throw UOE (like its siblings).

Ah, I missed those, good catch!

>> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/CallingSequenceBuilder.java line 134:
>> 
>>> 132:     private long computeImrSize() {
>>> 133:         return outputBindings.stream()
>>> 134:                 .filter(Binding.Move.class ::isInstance)
>> 
>> extra space here before `::`
>
> These are all nice stream calls - but I note that isIMR, computeIMRsize all need to to a pass on the output bindings. So the code is higher level, but we do an extra pass on the bindings. That said, we pay for it at MH creation, so I guess you considered this and decided we can live with this (I agree).

Yeah, the stream calls only happen once when linking, so I'm not really worried about it.

I think we might be able to speed up linking in the future by pre-generating some of the calling sequences into a file akin to VarHandleGuards. But, it would be platform specific, so might need to wire that into the build system.

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

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