Implement `MemoryFill` and `MemoryCopy` bulk memory ops #29

ggreif · 2021-12-17T10:17:24Z

Adding support for the two most popular bulk memory operations: MemoryFill and MemoryCopy. Imported test suite in a newer version that also exercises these new instructions. Added an exclusion list for tests from the new suite that would need features that winter doesn't support yet, and kept the old suite wholesale.

Stuff to do:

bump version? — yes: 1.1.0
add other bulk ops
in src/Wasm/Syntax/Memory.hs: type Offset = Int32 should this be Nat32?
check for cnt > 0 in MemoryFill, MemoryCopy
implement directionality in MemoryCopy
write tests
why is elem.wast failing from the MVP suite? — it does~~n't~~ seem to be related.
what are the test/wasm and test/wast directories for? — I have nuked them for now.

also corrected textual syntax for `Memory{Size,Grow}` to be conforming with https://webassembly.github.io/spec/core/text/instructions.html#memory-instructions

does not follow https://webassembly.github.io/spec/core/exec/instructions.html#xref-syntax-instructions-syntax-instr-memory-mathsf-memory-fill yet

not adhering to https://webassembly.github.io/spec/core/exec/instructions.html#xref-syntax-instructions-syntax-instr-memory-mathsf-memory-copy yet

ggreif · 2021-12-17T13:15:16Z

@nomeata I am not really in the know about how close we have to follow the spec and how to test new instructions. Can you have a look at what I have now and give some guidance about how to test this?

nomeata · 2021-12-17T14:30:00Z

We can run the official test suite. Does that not include tests for these instructions?

this is shamelessly frobbed from https://github.com/WebAssembly/testsuite/blob/main/bulk.wast (the official test suite) but shortened to only those two instructions

ggreif · 2021-12-18T11:29:50Z

I found the official tests too, but like 66c7510 they contain assert_return and other directives, and I cannot manage to run the interpreter with those in the script. E.g.:

[nix-shell:~/winter/test/wast]$ wat2wasm --enable-bulk-memory memory_copy.wast -o ../wasm/memory_copy.wast
memory_copy.wast:15:1: error: unexpected token (, expected EOF.
(invoke "test")
^

Is there a special trick, or another tool to run these?

nomeata · 2021-12-18T13:14:22Z

Yes, these are wast scripts, that's a superset of wat. There is a test runner for them included somewhere in the test suite of winter (which runs the official test suite, maybe if you set an envvar), though. I paged out most of the details.

src/Wasm/Exec/Eval.hs

src/Wasm/Binary/Decode.hs

nomeata

There are Wasm files committed; intentionally?

I assume the code was ported, where applicable, from the ocaml reference implementation? (This is desirable as it makes adding more feature in the future easier.)

ggreif · 2021-12-19T11:26:27Z

I have blown away all that stuff (but I could bring it back if there is a good reason for it). My hunch is that it was needed while original development. Now running nix-build will do the job. If I would feel the desire to look at the Wasm files I'd just modify the Wat2Wasm.hs to keep them.

nomeata

Ok, I must have misread the github diff, it looked as if some binary files were chnaged but not deleted.

default.nix

Co-authored-by: Bas van Dijk <bas@van.dijk.ch>

ggreif · 2021-12-20T13:12:36Z

The elem.wast fails like this:

    /nix/store/jvzss7p21054dmbjqlbg01ya1h4nzx06-source/elem.wast:                   FAIL
      line 4
        user error (./test2194-1528.wat:13:4: error: redefinition of elem "$t"
          (elem $t (i32.const 0) $f $f)
           ^^^^
        ./test2194-1528.wat:14:4: error: redefinition of elem "$t"
          (elem $t (offset (i32.const 0)))
           ^^^^
        ./test2194-1528.wat:15:4: error: redefinition of elem "$t"
          (elem $t (offset (i32.const 0)) $f $f)
           ^^^^
        )

But I don't think this is related to my changes.

But somehow it is different! When running from origin/master:

[nix-shell:~/winter]$ env WASM_SPEC_TESTS=$(nix-build -A WASM_SPEC_TESTS) cabal test

I get

Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - winter-1.0.0 (test:wasm-test-suite) (first run)
Preprocessing test suite 'wasm-test-suite' for winter-1.0.0..
Building test suite 'wasm-test-suite' for winter-1.0.0..
Running 1 test suites...
Test suite wasm-test-suite: RUNNING...
Test suite wasm-test-suite: PASS
Test suite logged to:
/Users/ggreif/motoko/winter-WASM/dist-newstyle/build/x86_64-osx/ghc-8.10.7/winter-1.0.0/t/wasm-test-suite/test/winter-1.0.0-wasm-test-suite.log
1 of 1 test suites (1 of 1 test cases) passed.

Mystery resolved: it is due to the --enable-bulk-memory flag. wat2wasm becomes more picky with it. 4817a69 fixes this.

…ding `--enable-bulk-memory` by making it possible to pass language flags per suite

nomeata · 2021-12-21T14:40:31Z

src/Wasm/Exec/Eval.hs

+        (MemoryFill, I32 0 : _ : I32 dst : vs') -> {-# SCC step_MemoryFill #-} do
+          inst    <- getFrameInst
+          mem     <- lift $ memory inst (0 @@ at)
+          -- Zero len with offset out-of-bounds at the end of memory is allowed
+          sz      <- lift $ lift $ Memory.size mem
+          if Memory.pageSize * sz >= dst
+            then k vs' es
+            else k vs' (Trapping (memoryErrorString Memory.MemoryBoundsError) @@ at : es)
+
+        (MemoryFill, I32 cnt : v : I32 dst : vs') -> {-# SCC step_MemoryFill #-} do
+          inst    <- getFrameInst
+          mem     <- lift $ memory inst (0 @@ at)
+          let [addr, count] = fromIntegral . i64_extend_u_i32 . fromIntegral <$> [dst, cnt]
+          if addr + count > 2^32
+            then k vs' (Trapping (memoryErrorString Memory.MemoryBoundsError) @@ at : es)
+            else do
+              eres <- lift $ lift $ runExceptT $ mapM_ (\off -> Memory.storePacked Pack8 mem addr off v) [0 .. pred cnt]
+              case eres of
+                Right () -> k vs' es
+                Left exn -> k vs' (Trapping (memoryErrorString exn) @@ at : es)


This looks quite different from the reference code in
https://github.com/WebAssembly/spec/blob/ffb5e3b40db7a45237d4e2fc972c1552ac4abef5/interpreter/exec/eval.ml#L413-L428
(and likewise below)

The design of winter is to be a pretty much direct port, so that chances are good to get the semantics right, and also to be able to follow and apply subsequent changes easily.

We have occasionally deviated, for performance reasons (e.g. https://www.joachim-breitner.de/blog/765-Faster_Winter_7__The_Zipper). Is this the reason here? Maybe worth adding a comment here… hmm, but I see you aren’t still implementing this to byte-wise operations here.

So why not just follow the logic of the reference code directly?

Because I didn't even look at the other reference :-)

Will do.

Looking at that reference implementation makes me think it is written with proof assistants in mind. It lowers the complex instructions to simple ones and stucturally "smaller" identical ones. There is a case of MemoryCopy that isn't even tail recursive. So to make this remotely performant we'll have to rewrite these anyway. Otherwise even the Rust-style "do prefix and suffix bytewise and the inner part u64-wise" will be executing faster.

nomeata · 2021-12-21T14:43:21Z

test/Main.hs

+  putStrLn $ "Using wasm MVP spec test directory: " ++ testDirMVP
  putStrLn $ "Using wasm spec test directory: " ++ testDir


Why do we need two versions of the test suite? Can we not simply run the latest, possibly excluding new tests that we don't support?

Because I had to disable too many tests that won't run in the new suite, and became concerned about regressing on those.

WIP: decode some Memory*

5ca4076

ggreif force-pushed the master branch from 705ff01 to 5ca4076 Compare December 17, 2021 10:21

ggreif added 2 commits December 17, 2021 11:39

administrative impl

59e7ffc

also corrected textual syntax for `Memory{Size,Grow}` to be conforming with https://webassembly.github.io/spec/core/text/instructions.html#memory-instructions

encoding and DSL

474460c

ggreif changed the title ~~Decode some Memory* ops~~ Implement MemoryFill and MemoryCopy bulk memory ops Dec 17, 2021

ggreif force-pushed the master branch from 4ff01fd to 591fbc9 Compare December 17, 2021 12:12

ggreif mentioned this pull request Dec 17, 2021

Gabor/memcpy caffeinelabs/motoko#3009

Closed

first shot at MemoryFill semantics

306b0c6

does not follow https://webassembly.github.io/spec/core/exec/instructions.html#xref-syntax-instructions-syntax-instr-memory-mathsf-memory-fill yet

ggreif force-pushed the master branch from 591fbc9 to cd54a93 Compare December 17, 2021 12:40

implement semantics for MemoryCopy

5c7c12f

not adhering to https://webassembly.github.io/spec/core/exec/instructions.html#xref-syntax-instructions-syntax-instr-memory-mathsf-memory-copy yet

ggreif force-pushed the master branch 2 times, most recently from cb56cce to e3d154d Compare December 17, 2021 12:59

touchups

ca0574c

ggreif force-pushed the master branch from e3d154d to ca0574c Compare December 17, 2021 13:05

implement memory presence test for MemoryFill

993f4c3

ggreif force-pushed the master branch from ed102f6 to 993f4c3 Compare December 17, 2021 13:41

check addresses for 0-size MemoryCopy

b814f13

ggreif force-pushed the master branch from e2e582e to b814f13 Compare December 17, 2021 13:56

implement directionality for MemoryCopy

53203cb

add tests for memory.fill and memory.copy

66c7510

this is shamelessly frobbed from https://github.com/WebAssembly/testsuite/blob/main/bulk.wast (the official test suite) but shortened to only those two instructions

pull in more tests (also bulk memory ones)

8af6347

ggreif force-pushed the master branch from 485f743 to 8af6347 Compare December 18, 2021 13:52

ggreif added 4 commits December 18, 2021 15:16

typo

3d25da0

moderenise

c1fbd74

excude bulk.wast for now

48618fb

accept bulk-memory instrs

6df7ecb

validate src too

846bbf2

ggreif force-pushed the master branch from 5b72b24 to 846bbf2 Compare December 19, 2021 01:14

ggreif commented Dec 19, 2021

View reviewed changes

src/Wasm/Exec/Eval.hs Outdated Show resolved Hide resolved

ggreif commented Dec 19, 2021

View reviewed changes

src/Wasm/Exec/Eval.hs Outdated Show resolved Hide resolved

ggreif added 4 commits December 19, 2021 02:25

simplify

d99970c

simplify zero-range MemoryCopy too

1cf9f32

remove test/wasm and test/wast

07673a9

elim warning

a9b38a4

ggreif commented Dec 19, 2021

View reviewed changes

src/Wasm/Binary/Decode.hs Outdated Show resolved Hide resolved

ggreif commented Dec 19, 2021

View reviewed changes

src/Wasm/Binary/Decode.hs Outdated Show resolved Hide resolved

ggreif added 2 commits December 19, 2021 03:02

Update src/Wasm/Binary/Decode.hs

355c6fc

Update src/Wasm/Binary/Decode.hs

96e5a29

ggreif marked this pull request as ready for review December 19, 2021 02:10

nomeata reviewed Dec 19, 2021

View reviewed changes

nomeata approved these changes Dec 19, 2021

View reviewed changes

ggreif force-pushed the master branch from 3654c2b to 96e5a29 Compare December 19, 2021 11:50

basvandijk reviewed Dec 20, 2021

View reviewed changes

default.nix Outdated Show resolved Hide resolved

ggreif and others added 2 commits December 20, 2021 10:57

Update default.nix

338cdec

Co-authored-by: Bas van Dijk <bas@van.dijk.ch>

simplify

5373bf2

basvandijk approved these changes Dec 20, 2021

View reviewed changes

ggreif added 2 commits December 20, 2021 15:50

Merge branch 'master' of https://github.com/dfinity-side-projects/winter

04057a0

repair the problem which happened with elem.wast (from MVP) when ad…

4817a69

…ding `--enable-bulk-memory` by making it possible to pass language flags per suite

ggreif requested a review from nomeata December 20, 2021 18:20

bump version

220b7ee

ggreif force-pushed the master branch from 0ae475d to 220b7ee Compare December 20, 2021 18:27

update URLs

618f679

nomeata reviewed Dec 21, 2021

View reviewed changes

ggreif mentioned this pull request Dec 6, 2022

Activate bulk memory in the Rust-compiled code caffeinelabs/motoko#3621

Closed

		putStrLn $ "Using wasm MVP spec test directory: " ++ testDirMVP
		putStrLn $ "Using wasm spec test directory: " ++ testDir

Implement MemoryFill and MemoryCopy bulk memory ops #29

Are you sure you want to change the base?

Implement MemoryFill and MemoryCopy bulk memory ops #29

Uh oh!

Conversation

ggreif commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ggreif commented Dec 17, 2021

Uh oh!

nomeata commented Dec 17, 2021

Uh oh!

ggreif commented Dec 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomeata commented Dec 18, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nomeata left a comment

Choose a reason for hiding this comment

Uh oh!

ggreif commented Dec 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomeata left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ggreif commented Dec 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomeata Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

ggreif Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreif Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nomeata Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

ggreif Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement `MemoryFill` and `MemoryCopy` bulk memory ops #29

Implement `MemoryFill` and `MemoryCopy` bulk memory ops #29

ggreif commented Dec 17, 2021 •

edited

Loading

ggreif commented Dec 18, 2021 •

edited

Loading

ggreif commented Dec 19, 2021 •

edited

Loading

ggreif commented Dec 20, 2021 •

edited

Loading

ggreif Dec 21, 2021 •

edited

Loading

ggreif Feb 13, 2023 •

edited

Loading

ggreif Dec 21, 2021 •

edited

Loading