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

Add alternate proposal for segment init. #79

Merged
merged 3 commits into from
Nov 18, 2017
Merged

Conversation

binji
Copy link
Member

@binji binji commented Nov 15, 2017

See the discussion in issue #62 starting here.

This solution introduces new instructions to initialize segments programmatically, but only in the start function.

This solution introduces new instructions to initialize segments programmatically, but only in the start function.
@lukewagner
Copy link
Member

Thanks for writing this up! A few high-level thoughts:

  • it seems useful to have the destination offset of init_table/init_memory be a dynamic operand (rather than being specified as the init_expr in the data section)
    • if we do this, then the init_expr operand of a data segment only has meaning when the segment is marked "active" and thus we should probably not even have that field present when "inactive"
  • I can also imagine it being useful (and no real challenge) to additionally have a second length operand to init_table/init_memory that specifies how much of the data segment to copy

[instantiation](https://webassembly.github.io/spec/exec/modules.html#instantiation).
Similarly, `init_table` has behavior matching step 10.

These instructions may only be used in the [start
Copy link
Member

Choose a reason for hiding this comment

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

Since finding out whether the code is running during instantiation requires a runtime check anyway it doesn't actually seem necessary to distinguish the start function syntactically -- and in fact, it might want to call helpers, for example.

The simplest semantics would come down to this: an instance has two states: initialising vs initialised. After the start function has returned the instance transitions from former to latter. In the second state the init instructions trap.

@rossberg
Copy link
Member

Agreed with @lukewagner. I'd go even further and say that these instructions should work like memcopy, i.e., have the following arguments:

  • source segment (immediate)
  • target table/memory (immediate)
  • source offset (i32 arg)
  • target offset (i32 arg)
  • length (i32 arg)

@lars-t-hansen
Copy link

Agreed with @lukewagner and @rossberg on their points. Making these instructions act like memcpy provides maximal flexibility and there does not seem to be any significant downside to that.

@jfbastien
Copy link
Member

The instruction would just be a "copy to memory from read-only segment"? Won't really have anything to do with initialization anymore. That's kinda neat. I assume we'll spec the writes as non-atomic, ordered from low to high bits?

@binji
Copy link
Member Author

binji commented Nov 17, 2017

Addressed some of the comments, is this clear enough or should I go into more detail?

I assume we'll spec the writes as non-atomic, ordered from low to high bits?

@jfbastien Yes, we already have similar wording here, I imagine we'll just have a spec "function" that shares that behavior.

Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

lgtm with one comment, thanks!

target offset, and length as given by the `init_memory` operands.

A trap occurs if any of the accessed bytes lies outside the source data segment
or the target memory.
Copy link
Member

@lukewagner lukewagner Nov 18, 2017

Choose a reason for hiding this comment

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

I think the "trap if not during the call-start-function phase of instantiation" rule seems to have gone away.

@binji binji merged commit 4b0f677 into master Nov 18, 2017
@binji binji deleted the cond-segment-instr branch November 18, 2017 18:35
@jfbastien
Copy link
Member

Why is "trap if not during the call-start-function phase of instantiation" desirable?

@lukewagner
Copy link
Member

See comment in #62 and following.

@jfbastien
Copy link
Member

See comment in #62 and following.

That seems like an odd restriction from a user perspective, and is odder to implement than the alternative. Even the optimization where we throw away now-dead segments isn't that hard if start is the only caller and isn't exported (call graph not needed, just "is called" bit on each function).

@lukewagner
Copy link
Member

Yes, but that means there will be an implicit memory usage cliff that could be triggered by something as arbitrary as your C++ compiler's inlining heuristics. The implementation is quite trivial (it's certainly more work to do heuristics that best-effort discard segments) and the restriction isn't odd when the performance situation is explained (as is the case for much of wasm).

@rossberg
Copy link
Member

Agreed it is kind of odd, but it is conservative -- we could lift the restriction later. Alternatively, we could extend the segment flag to explicitly indicate whether a given segment should be usable after initialisation.

@jfbastien
Copy link
Member

I think we should explicitly discuss this at an upcoming meeting.

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.

5 participants