clarify that indirect calls cannot call imports

[MikeHolman]

To help engines optimize calls, we should know ahead of time whether call is to import or not (e.g. to avoid needing intermediate thunk for boxing).

[lukewagner]

lgtm

FWIW, a change to BinaryEncoding.md I've prototyped/implemented in SpiderMonkey and was planning to propose puts imports into their own section so that "indices into the function section" necessarily means "not an import". I think this is a good change for a number of reasons:

• the imports section can be moved to the very beginning of the module (only preceded by the signature section) allowing streaming es6 module loading to more eagerly issue fetches for imports
• imports and functions have separate index spaces; putting them in separate sections makes this clear
• in the future we'll likely want to allow importing more than functions (e.g., linear memories) and having a separate section allows these to be added in a regular way
• symmetric with proposal to put exports in their own section, as discussed in spec/#243

[jfbastien]

The basic proposal seems to work great with single module calling imports from the embedder (it's easy for the linker to add the required thunks). I'm not sure it works when we add dynamic linking.

Say module A has class Base, and module B has class Derived, and I dynamically import B, its functions, and vtable. From A I call a Base* which is actually a Derived*, load from its vtable and call the indexed function. How do I know it's an import?

Can this be added without breakage post-MVP?

[titzer]

I already have a CL out to add this to V8:
https://codereview.chromium.org/1709653002/

Having implemented both now, I still think a combined index space for
functions is better, since V8 simply puts a pointer the adapter code into
the indirect function table and it Just Works(TM). I think it's actually
simpler and more efficient if the engine implements this rather than
forcing the module to do that. Nevertheless I'll probably land the above CL
to defer to the consensus here.

On Fri, Feb 19, 2016 at 7:53 AM, JF Bastien [email protected]
wrote:

The basic proposal seems to work great with single module calling imports
from the embedder (it's easy for the linker to add the required thunks).
I'm not sure it works when we add dynamic linking.

Say module A has class Base, and module B has class Derived, and I
dynamically import B, its functions, and vtable. From A I call a Base*
which is actually a Derived*, load from its vtable and call the indexed
function. How do I know it's an import?

Can this be added without breakage post-MVP?

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

[lukewagner]

@jfbastien The important thing to remember is that imports are not the logical equivalent of __declspec(dllimport); they're more like syscalls; they "leave" (syscalls leave the process, call_import leaves the instance). While it is normal to take the address of a dllimport and call that indirectly, no one expects to take the address of __syscall(sys_write). I'm pretty sure we'll want to add something just like you're imagining as part of the dynamic linking feature (and I expect the right place for that new dllimport is in the function section as a second kind of function (b/c dllimports would be part of the function index space); so not that different than @titzer's original design with imports).

[jfbastien]

@lukewagner ah yes, this sounds good.

[ghost]

A. Which section imported functions are placed in appears orthogonal to them being callable by an indirect call, so it's not clear if any of the discussion related to sections is relevant.

B. The rationale 'to help engines optimize calls' appears to be refuted by current experience. Worst case the engine needs to emit the stubs rather than require producers to always do so.

C. An analogy to 'syscalls' does not appear very useful, and perhaps easily refuted. For example, the wasm code calls the import 'request_sys_op_access' with a code for the operation, and only if granted then runtime adds an entry to the vtable and returns the index - the wasm code can not even call the restricted operation if not granted use and perhaps this fits some security model. Is this example possible, or is there some show stopper that would block this example usage, and might understanding this help understand this issue?

D. A goal of wasm seems to be to make the hardware performance available. If a trampoline can be avoided then eliminating it seems consistent with this goal. Adding artificial restrictions just to borrow one particular model might not be consistent with this goal.

Just suggesting that the rationale might not be well articulated, or that it's a little beyond me to understand yet and requesting it be communicate in simpler terms, or some more discussion.

[MikeHolman]

I can give a little more context. Currently, when a call is to an import, we (Chakra) jit code to do boxing of arguments inline in the caller and then call the normal js entrypoint. Same for unboxing return value.

If we don't know whether call is to an import, we will need to set the entrypoint for imports in the indirect function table to be some intermediate thunk which does this, because the js entrypoint expects js vars, not raw ints/floats.

If there is good motivation for indirect calls on imports and we put these in different table, then we could by construction know whether to jit code for boxing or not, and that would also be acceptable solution for me. However, I don't know if this be an issue for wasm producers.

[ghost]

Thank you, so might the following be closer to the rationale?

There is a small overhead in complexity for the runtime to build trampolines (intermediate thunks) for indirect calls to imports but currently there appears to be not use case for indirectly calling imports so they are excluded. If in future there were a use case then imports could be assigned an index too.

[MikeHolman]

Based on this conversation, it seems this may be a useful clarification regardless of whether there is a separate imports section?

[ghost]

@MikeHolman Can you propose an addition to the rationale to explain it?

[sunfishcode]

With #682, the design now specifies that indirect calls can call imports.