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

Define a start method #495

Merged
merged 1 commit into from
Dec 12, 2015
Merged

Define a start method #495

merged 1 commit into from
Dec 12, 2015

Conversation

jcbeyler
Copy link

@jcbeyler jcbeyler commented Dec 8, 2015

Adding the definition of a start method that would be a top-level module statement.

@jcbeyler jcbeyler mentioned this pull request Dec 8, 2015
@jfbastien
Copy link
Member

IIUC this isn't the same thing as what @lukewagner suggested in #398 (comment) ?

@jcbeyler
Copy link
Author

jcbeyler commented Dec 9, 2015

Yes exactly, he asked me to do a PR, so here it is: #398 (comment)

@lukewagner
Copy link
Member

lgtm, thanks!

@jfbastien
Copy link
Member

I expected something more concrete, mentioning (start $func) or something of the kind.

@jcbeyler
Copy link
Author

Added an example for JF


or

```(start 0)```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what this would do?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done I think :)

@jfbastien
Copy link
Member

lgtm with small explanation. Thanks!

jfbastien added a commit that referenced this pull request Dec 12, 2015
@jfbastien jfbastien merged commit f92c671 into WebAssembly:master Dec 12, 2015
jfbastien added a commit to WebAssembly/binaryen that referenced this pull request Feb 4, 2016
As spec'd in: WebAssembly/design#495
And discussed in: WebAssembly/spec#231

This will make it simpler and more uniform to add a start entry point.

s2wasm is the right place to add start because it'll eventually need to
do other basic setup, e.g. put code in start to setup the stack, as
dschuff is doing in:
  #179
Or rather, the linker is the right place and s2wasm happens to act as
our linker right now.
jfbastien added a commit to WebAssembly/binaryen that referenced this pull request Feb 5, 2016
As spec'd in: WebAssembly/design#495
And discussed in: WebAssembly/spec#231

This will make it simpler and more uniform to add a start entry point.

s2wasm is the right place to add start because it'll eventually need to
do other basic setup, e.g. put code in start to setup the stack, as
dschuff is doing in:
  #179
Or rather, the linker is the right place and s2wasm happens to act as
our linker right now.
@dschuff
Copy link
Member

dschuff commented Apr 1, 2016

We've started some discussion about how this should integrate with emscripten in emscripten-core/emscripten#4218

It's not really clear from the current text in the design repo how it should work with the web embedding. e.g. when you instantiate the module, is the VM expected to call the start function automatically (if so, when? before Wasm.instantiateModule returns?), or should that be done by the enclosing JS code?

I think it should be put under the control of the JS code. For example (as a strawman) the instance object returned by Wasm.instantiateModule could have a way to determine which function is the start function, and then the JS would have to call that function before calling anything else on the instance. Or just have a method with the same name on every instance that is called by the JS, which itself calls the start function. Or whatever. The point is that it would allow the JS to instantiate the module, then modify the linear memory and do whatever JS wrapper stuff it would need to, then call the wasm start function (which would presumably then call its own global initializer functions).

@dschuff
Copy link
Member

dschuff commented Apr 1, 2016

oh oops, I meant to post this in #398, not in this PR.

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

Successfully merging this pull request may close these issues.

None yet

4 participants