Skip to content
This repository was archived by the owner on Nov 3, 2021. It is now read-only.

[interpreter] Use small-step semantics #115

Merged
merged 18 commits into from
Oct 7, 2019
Merged

[interpreter] Use small-step semantics #115

merged 18 commits into from
Oct 7, 2019

Conversation

rossberg
Copy link
Member

Implement bulk memory operations in small-step semantics, as in spec.

Minor fixes:

  • In spec, memory.copy was missing use of \vconst in the backwards case
  • In interpreter & test, memory.init still trapped if segment has been dropped but length parameter was 0.

Baseline is #114. See last commit for new changes.

@rossberg rossberg requested a review from binji September 29, 2019 15:03
@rossberg rossberg mentioned this pull request Oct 1, 2019
Copy link
Contributor

@gahaas gahaas 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 nits.

{ty = I32Type; align = 0; offset = 0l; sz = Some Memory.Pack8});
]

| MemoryCopy, I32 n :: I32 s :: I32 d :: vs' when s >= d ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The text spec says dst<=src. you could change the order of the comparison here to make the implementation here match the text spec better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member Author

@rossberg rossberg left a comment

Choose a reason for hiding this comment

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

Thanks for all the reviews!

@rossberg rossberg mentioned this pull request Oct 4, 2019
@rossberg rossberg merged commit 8db23a9 into master Oct 7, 2019
@rossberg rossberg deleted the smallstep branch October 7, 2019 09:53
@AndrewScheidecker
Copy link
Contributor

In interpreter & test, memory.init still trapped if segment has been dropped but length parameter was 0.

Are you planning to make the same change for table.init?

@rossberg
Copy link
Member Author

rossberg commented Oct 7, 2019

@AndrewScheidecker, yeah, there were todos for redoing the table instructions, but fixing the outdated tests seems more urgent, so see #120 .

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

Successfully merging this pull request may close these issues.

3 participants