[foreign-memaccess+abi] RFR: 8255903: Enable multi-register return values for native invokers [v3]
Jorn Vernee
jvernee at openjdk.java.net
Thu Oct 28 13:22:32 UTC 2021
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
More information about the panama-dev
mailing list