Enable Rust/C functions to "hook up" to multi-value functions

[alexcrichton]

Over in wasmtime I've been exploring the area of hooking up compile-time functions (those defined in Rust or C) directly with cranelift-generated functions. For example a wasm module like this:

(module
  (import "" "" (func (param i32)))
  (start (func 0)))

could get hooked up directly to a Rust function that looked like:

extern "C" fn the_import(vmctx: *mut VMContext, param: i32) { /* ... */ }

(more or less). When I say "hooked up directly" here I mean that zero cranelift-generated shim functions are required to have cranelift call out to the imported function. Today the wasmtime::Func type always uses cranelift to generate a fresh jit function which has an appropriate trampoline.

The goal here is to basically make extraction of a function pointer and calling it or providing a function pointer as an import as cheap of an operation as possible.

This all works well today with the default ABIs implemented in Cranelift and in general we don't have any issues. For example #839 is a start of how this might look in the wasmtime crate.

A wrench is thrown into the works with multi-value wasm functions, however. The ABI of a multi-value return function is fundamentally incompatible with anything you can write in C/Rust/etc today. In talking with @sunfishcode it looks like the multi-value return ABI (when dealing with more than one return value) is modeled after the concept of a multi-value return in LLVM. This is, however, purely an aspect of LLVM IR where you can return a tuple, and you can't actually write stable Rust or C code to generate LLVM IR that has this form of a tuple return. This means that if you have a module like:

(module
  (import "" "" (func (result i32 i32)))
)

it's fundamentally impossible to provide a Rust/C function pointer as the import there. There's no way to get the compiler to generate a function that supports the ABI that cranelift expects, meaning that cranelift is required to generate a jit function shim to call between Rust and wasm.

In some discussion, I think there's two possible ways to fix this:

Change Cranelift to behave as if it's returning a struct, not a tuple

Currently cranelift's multi-value return ABI is modeled after what the multi-return ABI looks like in LLVM/gcc. While there is not source-language-level equivalent to this it's all compiler-internal details and you can generate LLVM IR to match up with what cranelift generates today.

An alternative, though, is to update Cranelift's interpretation of a multi-value return to "behave as if an aggregate struct was returned with the multi-value fields" if there is more than one return value. This feels a bit weird, but for example given:

(module
  (import "" "" (func (result i32 i32)))
)

you could hook that up natively to:

#[repr(C)]
struct A(i32, i32);

extern "C" fn foo(vmctx: *mut VMContext) -> A {
    // ... 
}

Or perhaps instead of changing cranelift's default we could simply add a new ABI which matches a "struct-like return" ABI rather than the current tuple-like return ABI.

In any case this seems like it would be a significant undertaking. The rules for how to return an aggregate are really complicated and at least to me make basically no sense. Trying to have Cranelift match exactly what the system ABI looks like is likely a very large undertaking which would take quite some time to get right (and likely a bunch of internal refactoring).

This leads us to another alternative...

Change cranelift to always use an out-pointer for multi-value returns

Instead of trying to match exactly what the system ABI looks like for returning aggregates, we could change cranelift's system ABI (or add a new ABI, or just do this all at the wasm layer) to do something like follows:

• If a function returns 0 values or 1 value, do the normal thing you'd expect (match the system ABI)
• If a function returns 2 or more values, then always synthesize an out-pointer where the layout of the out-pointer is the same as a C struct with fields of the types of the return value.

This way the above example could be hooked up to a function like this:

#[repr(C)]
struct A(i32, i32);

extern "C" fn foo(outptr: *mut A, vmctx: *mut VMContext) {
    // ... 
}

The downside of this approach is that it may be wasm-specific or a bit of a hack in cranelift, but the upside is that there's no mucking around with the system ABI and trying to do clever things like packing (i32, i32) into a 64-bit register.

This feels like the better solution to me, but I'm curious to hear what others think as well! If others have questions I certainly don't mind answering them as well.

[bjorn3]

If a function returns 2 or more values, then always synthesize an out-pointer where the layout of the out-pointer is the same as a C struct with fields of the types of the return value.

For exactly two return values, cg_clif needs to pass both in registers to match the native abi. This is what is currently implemented for the SystemV abi.

[bnjbvr]

Change cranelift to always use an out-pointer for multi-value returns

I like this approach because it would be nice and simple to do so. Cranelift has several use cases, though, and it would create a privilege for a given use case to only adopt one possible way of returning multiple values; other implementations would need thunks to adapt to this ABI, adding a performance penalty.

So I'm all in favor of implementing this in addition to the current thing (that could be called "systemv_multiple_results"), if you want to simplify the way it's done in Wasmtime.

Spidermonkey does its own thing, and this is subject to change in the future too: if it's a multiple results function, the first result goes into a register, and all the following will go onto the stack. So we will have this requirement of having several multiple-returns ABI specifications anyway.

[alexcrichton]

@bnjbvr oh right sorry I realize now how I was saying that things should outright change, but what I meant and a much better way to word it is how you put it, adding support for a different ABI. That'd definitely work well for wasmtime I think!

[bnjbvr]

@alexcrichton No worries! This plan sounds good.

[fitzgen]

The other wrinkle is that by the time we allocate a struct return or not, we don't really know if the values we're returning are "actually" multi-value or simply got split during legalization into multiple values.

A dedicated ABI for always using return pointers seems like a good plan, and should make dealing with the above easier.

[nlewycky]

We're looking into implementing multi-value return with the same ABI as a C struct return for our wasm implementation.

// We pack the values.
#[repr(C)]
pub struct S2 ( i32, i32 );
extern fn test1() -> S2 {
    S2(1, 2)
}

// Needs to be returned sret, but cranelift returns RAX, RDX, RCX.
#[repr(C)]
pub struct S3 ( i64, i64, i64 );
extern fn test2() -> S3 {
    S3(1, 2, 3)
}

The way I've done it so far is that the user of cranelift is responsible for doing packing and unpacking of pairs of 32-bit value into a single 64-bit value (including writing out a different function signature if needed, for example, a wasm signature returning I32, F32, I32 becomes a cranelift signature that returns i64, i32), while cranelift is responsible for doing sret conversion when needed. Unfortunately, cranelift doesn't always perform sret conversion when the equivalent C struct would be returned sret.

This means that either the caller also needs to perform sret conversion in the remaining cases, or we can write a patch that adds a new system_v_multiple_results ABI to cranelift. The new ABI would disable usage of RCX as a return and change when cranelift applies sret, but would not resolve the issue with packing and unpacking of values because there is currently no way for legalize_args to assign two return values into one register (ValueConversion::IntSplit does the opposite, splitting a single value across multiple registers). Would you be interested in a patch which adds that?

[sunfishcode]

I would like Cranelift to avoid evolving in the direction of C-compatible struct calling convention support. A full implementation, especially on x86-64, would require fairly detailed knowledge of the C type system, which is otherwise out of scope for Cranelift, so most likely we'd end up with partial support, which is awkward in terms of users knowing what to expect, developers knowing what features we intend to support and what's out of scope, and awkwardly overlapping functionality with the full support we hope to add in Cranelift frontends in the future.