Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic invoke/dynCall #240

Closed
nhynes opened this issue Mar 6, 2019 · 6 comments
Closed

Dynamic invoke/dynCall #240

nhynes opened this issue Mar 6, 2019 · 6 comments
Labels
🎉 enhancement New feature! ❓ question I've a question!

Comments

@nhynes
Copy link

nhynes commented Mar 6, 2019

Currently wasmer requires a bespoke definition for each invoke/dynCall. This is somewhat non-ideal since the number of invoke_[vid]+ functions is countably infinite. I took a whack at a DynFunc version of invoke, but fn import_functions returns ImportedFunc, which won't comfortably hold a DynFunc. It'd be nice to see a clean way to do this!

@Hywan Hywan added 🎉 enhancement New feature! ❓ question I've a question! labels Mar 6, 2019
@xmclark
Copy link
Contributor

xmclark commented Mar 6, 2019

You are too nice! The current solution is stop-gap and not ideal. We could come up with clever and equally elegant solution that handles all the edge cases.

Another idea we had was to make the code less verbose with macros and other code gen. It would still be static, but it would be a lot nicer to look at.

If you think doing a dynfunc is best, we would be happy to work with you on it! PRs welcome!

@nhynes
Copy link
Author

nhynes commented Mar 6, 2019

Thanks for the quick response!

macros and other code gen

I'm always a fan of static dispatch when possible. What were you thinking here? I see three obvious options:

  1. Create a bunch of boxed closures when the module is loaded but before cranelifting. This would still require a dynamic type to store the closure cache.
  2. Use a proc macro to generate the power set of {v, i, d, j}^7, but this would make extra work for the compiler and then someone with invoke_iiiiiiii would just come along and ruin everything
  3. Parse the module, cargo compile a dylib containing the invokes, load it in wasmer and create ImportedFuncs. This option is probably the cleanest and most static if one ignores its patent absurdity :)

DynCall seems the most straightforward, but it'll take a bit of hacking to get the types and ownership ironed out.

@xmclark
Copy link
Contributor

xmclark commented Mar 7, 2019

That is a pretty good analysis! 😍

I was originally thinking of something along-the-lines of #2. Yes, it would hurt compile times, but I think the solution would have terse code, and we wouldn't have to introduce many new concepts into the code base.

Introducing a dynamic call would be cool, but it may introduce some complexity. We are currently working on a few other backends (including LLVM!) and this may slow progress there.

If you could imagine a clever way of doing dyncall, I am all hears 👂

In the mean time, I am going to think about it for a bit. Will get back to you 🔙 👌. It seems like this problem is important to you so, we should get it figured out!

@kanaka
Copy link

kanaka commented Apr 16, 2019

The way that this works in the wace interpreter is that import is basically a direct dlopen/dlsym. In other words, this works without any knowledge about fputs in wace:

(import "env" "fputs"       (func $lib_fputs (param i32 i32) (result i32)))

wace is even able to handle function pointer based callbacks (needed for SDL for example) by doing mprotect on the table data and then trapping on reads of the table and translating the offsets back into actual function pointers. And for 32-bit x86 it even supports calling functions that use 16bit and 8bit parameters but this is relying on the little endian nature of x86 and the way parameters are passed/padded on the stack so it wouldn't necessarily be portable.

One major caveat however is that this requires that the wace executable to be compiled as a 32-bit program.

Might or might not be relevant but I thought I would mention that this is already being done in a different context :-)

@syrusakbary
Copy link
Member

This is now solved in the refactor. Will close this issue once it lands in master.

@syrusakbary
Copy link
Member

Thanks to the refactor, now all functions (host and wasm) can be called dynamically (via Function::call).

Closing the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! ❓ question I've a question!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants