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

call_import separate from call_direct? #421

Closed
titzer opened this issue Oct 22, 2015 · 19 comments
Closed

call_import separate from call_direct? #421

titzer opened this issue Oct 22, 2015 · 19 comments
Milestone

Comments

@titzer
Copy link

titzer commented Oct 22, 2015

Should we combine the import table and the function table like v8-native-prototype, which allows combining call_direct and call_import and indirect calls to imports, or have separate tables?

@jfbastien
Copy link
Member

I kind of see the distinction the same way relocations work. I think we can optimize things at load time, but I'm not sure we want to mandate a specific implementation. Put another way: does this distinction give us something?

@lukewagner
Copy link
Member

My main argument earlier was that putting normal functions and imports into the same main function table would suggest allowing imports in the indirect function table which in turn would require some extra thunking complexity for call_indirect (at least in the impls of both @dschuff and odin). Revisiting this and rereading odin codegen just now, I see that I was mistaken and that, with a bit of tweaking, no extra thunks would be necessary to make this work, at least for odin. :-$

I also think the spec will get a bit more complex having main function table entries being either internal functions or imports, but that's not a strong argument. In return, we'd get re-exports (requested in #355) "for free" and you could call_indirect an import directly.

So despite my, ahem, spirited objection earlier, I'm now mildly in favor. ¯_(ツ)_/¯

@rossberg
Copy link
Member

I don't think it would be wise to unify the two. Even if it turns out to be easy to treat imports in the same manner as internal functions in some implementations we should not assume that that is the case for all implementations and environments hosting Wasm. We might be putting a potentially significant burden on others. Given that there isn't any real benefit in combining them, erring on the conservative side seems advisable.

@lukewagner lukewagner added this to the MVP milestone Oct 23, 2015
@lukewagner
Copy link
Member

An additional point: when we add dynamic linking and need to export unaliased mutable thread-local variables (viz., sp), the identity of the export matters (iiuc, just like exported vars in ES6 modules) so we will need to be able to directly export an import. (And if you can reexport thread-locals, then it'd be asymmetric not to allow re-exporting a function.) Yes, we could work around this by allowing two kinds of export declarations (export a module-defined function/thread-local or export an imported function/thread-local). We'd also need to provide an _import version of the (set|get)_thread_local ops (like we've done with call). To be conservative in the MVP, we could initially disallow import indices in the indirect func table.

@lukewagner
Copy link
Member

FWIW, I'm back to thinking it's cleaner and simpler to keep things as-is in the spec. In addition to simplifying the impl (at least in SM), it's become more clear that the right way to think about imports, from a C++/compiler/toolchain pov, is as syscalls (e.g., they take you out of your "user" address space). Like syscalls, imports would be called in one or two places by wasm functions defined by toolchain/library. In particular, wasm imports do not correspond to dllexport/dllimport -- those would be supported by some new declaration as part of the dynamic linking post-MVP feature and those declarations would go into the function table and be callable through call/call_indirect (that's the whole point).

@kg
Copy link
Contributor

kg commented Jan 12, 2016

In that case, should we not call these imports anymore? Should we straight-up call them "syscalls"? That seems like it might be right. Then when we solve dynamic linking we introduce something tailor-made for those import/export scenarios that has the right semantics (and it would be ideal if you just used call/call_indirect for those).

@kripken
Copy link
Member

kripken commented Jan 12, 2016

Some (minor) confusion worries me about calling them syscalls, since libc ports might use that name for their current syscall interface, and it might not match up to what we call imports (some libc syscalls might end up implemented in wasm, such as a virtual filesystem).

@jfbastien
Copy link
Member

libc itself would use call_import to implement syscalls. I think the distinction @kripken makes is: where's the boundary for what the "WebAssembly OS" offers in a browser environment, and what's in .wasm files?

I think it's worth changing call_import to syscall, because it's really accessing the embedder the same way you call into an OS. I agree with @kripken that the boundary may shift over time, but it's still doing the same fundamental thing: asking the embedder to do a thing.

@lukewagner
Copy link
Member

Yeah, writing the above made me think about renaming too. I would like the name "syscall" except that it is a very monolithic-C++-app-centric view of the world. For all of the modular wasm-as-part-of-a-bigger-app use cases, then imports are aptly named. This isn't just on the web, but would show up in node.js and possibly other environments that wanted to allow wasm multiple instances for the memory-isolation benefits. Perhaps we could think of a less C++-centric name that still left "import" reserved for dynamic linking (say, extern/call_extern, except of course that C++ already has a separate meaning for extern)? If not, I was thinking that dynamic linking could use dllimport (and wouldn't need any new call_*) and the juxtaposition (import vs. dllimport) would make it clear which one did the C++ dynamic linking thing.

@ghost
Copy link

ghost commented Jan 12, 2016

How about splitting out the naming into a separate issue if there is no consensus now. It would be nice to have v8's binary implementation and the spec consistent if consensus can be reached now, irrespective of the naming.

@qwertie
Copy link

qwertie commented Jan 13, 2016

"syscall" sounds to me like calling into some kind of "system" (OS, browser, or other 'outer system'). How is it C++-centric, other than that C++ coders use the word more than others?

@rossberg
Copy link
Member

Note that these calls correspond to the "import" section of a Wasm module, so we should either rename both, or keep the name. Moreover, there also is the dualism with module "exports", which we also would want to keep somehow.

But FWIW, I tend to think of this mechanism as more of a foreign function interface. So how about call_foreign or call_extern?

@titzer
Copy link
Author

titzer commented Jan 13, 2016

If we add a call_multiple in the future to handle multiple return values,
will we then need to have a separate bytecode for each of call_direct,
call_indirect, and call_import? Perhaps we can use the signature, which
will indicate the number of return values.

On Wed, Jan 13, 2016 at 11:45 AM, rossberg-chromium <
notifications@github.com> wrote:

Note that these calls correspond to the "import" section of a Wasm module,
so we should either rename both, or keep the name. Moreover, there also is
the dualism with module "exports", which we also would want to keep somehow.

But FWIW, I tend to think of this mechanism as more of a foreign function
interface. So how about call_foreign or call_extern?


Reply to this email directly or view it on GitHub
#421 (comment).

@rossberg
Copy link
Member

@titzer: that's one of reasons why I think you don't want call_multiple, but something more compositional.

@kripken
Copy link
Member

kripken commented Jan 13, 2016

call_extern sounds ok to me. Then we'd have "internal calls" and "external calls".

But I also agree that it would be odd to lose the "import"/"export" parallel. So renaming call_import implies renaming imports, which implies renaming exports as well.

@kg
Copy link
Contributor

kg commented Jan 13, 2016

Are imports actually the dual of exports? @lukewagner 's suggestion was that dynamic linking between modules would probably use a different mechanism, and I think he might be right. In that case 'import' as it is right now is effectively a syscall, and 'export' is a mechanism used by the host environment to find the entry point (and in the case of wasm libraries, used by the host environment to access library APIs).
Those aren't directly linked together, in my opinion. If you wanted to keep the import/export terminology, perhaps they are 'host imports' and 'host exports', and dynamic linking imports/exports are contained within the wasm runtime environment. The host export scenario will also end up including some non-wasm glue to interface with the host environment in almost every case.

@lukewagner
Copy link
Member

@rossberg-chromium Maintaining import/export dualism is a good point: I liked extern, but I can't think of a good dual since extern seems just as well suited for describing imports as exports.

One idea is to call what we have in the wasm MVP "external imports"/"external exports" and, symmetrically, call the dynamic linking feature "internal imports"/"internal exports". Then you would call an external import with call_external (instead of call_import).

@lukewagner
Copy link
Member

@kg Since wasm environments (incl the web) can allow binding (to use my proposed terminology) external exports to external imports I do think there is a dualism. The discussion in the other issue is convincing me that "syscall", while effective as a metaphor for conveying the concept of "leaving the address space", incorrectly suggests that the call is into a system-fixed set of entry points, which isn't the case.

@sunfishcode
Copy link
Member

With the new design for tables established in #682, the there is no longer a distinct call_import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants