Integrated: 7903585: Revisit variadic support

Jorn Vernee jvernee at openjdk.org
Fri Dec 1 15:01:47 UTC 2023


On Fri, 1 Dec 2023 13:34:03 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Currently, we only support var args passed to a variadic function through an `Object[]`. From that, we infer the layouts of the arguments, and then link a variadic method handle on the fly.
> 
> This is problematic because: 1. there's an ambiguity between structs and pointers, since they both use `MemorySegment` as a carrier. 2. from a performance perspective, re-linking the method handle every time is somewhat costly.
> 
> The approach proposed by this patch is to add a variadic invoker factory method, which can, given a list of layouts, be used to create an instance of a variadic invoker (interface), that embeds a downcall handle specialized to a particular type of the variadic function. The user specifies the layout explicitly, so there is no ambiguity (1), and the user can cache the returned invoker and re-use it instead of re-linking the function on every invocation (2).
> 
> The current layout inferring method can then be re-implemented on top of this factory.
> 
> Question: I'm currently generating the doc comment on both the inferring method and the invoker factory. Is this good? Should it be on one or the other? Should it be on the invoker interface as well?
> 
> ---
> 
> There were some issues I ran into while developing this patch, that I fixed:
> - We had some methods in the tests that take no arguments, but were declared without a prototype, leading to them getting translated as variadic functions in the bindings. I've change those parameter lists to `(void)` (this popped out when I only had the invoker factory in an earlier iteration)
> - I noticed that jtreg was sometimes not recompiling a test class in the generator tests after the bindings changed, leading to weird NoSuchMethodErrors. I've fixed this by adding an explicit `@build` step to the tests.
> - I've added `-retain:fail,error` to the jtreg arguments, so that the generated sources for a failing test are not deleted immediately, and can be manually inspected after a test failure.
> - I noticed we weren't generating a layout for `C_BOOL`, because we don't treat that type as being supported. I think this is a latent issue. I've added Bool as a supported type to fix this.
> 
> Also: I've ret-conned Test8244959 and renamed it to TestPrintf. And added new cases for the invoker.

This pull request has now been integrated.

Changeset: 2fff774a
Author:    Jorn Vernee <jvernee at openjdk.org>
URL:       https://git.openjdk.org/jextract/commit/2fff774a6ed60f0e0f7fcb6bb9043543a1b63285
Stats:     455 lines in 39 files changed: 233 ins; 195 del; 27 mod

7903585: Revisit variadic support

Reviewed-by: mcimadamore

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

PR: https://git.openjdk.org/jextract/pull/149


More information about the jextract-dev mailing list