wasm2c: implement the bulk memory operations proposal #1877

keithw · 2022-03-31T21:40:48Z

~~(This PR is sequenced behind #1875 and #1814.)~~

Support bulk memory operations in wasm2c (the next step in the checklist at #1853 (comment)).

This PR also adds test coverage:

The current testsuite's elem.wast test (we had previously been using a pre-bulk-memory version in old-spec. These "old" tests are tracked at wasm2c no longer passes many spec tests #1737.)
The current testsuite's bulk.wast, memory_copy.wast, memory_fill.wast, and memory_init.wast (these were added in the bulk-memory proposal)
The current multi-memory proposal's load.wast and store.wast tests (we had previously been omitting these because they require bulk-memory support, discussed at wasm2c: run multi-memory tests #1834. Another multi-memory tests requires reference-types support, and the final multi-memory test requires SIMD.)
Bulk-memory proposal versions of the imports.wast, linking.wast, table_copy.wast, and table_init.wast tests. (The current versions of these are waiting on reference-types support.)

binji

lgtm! Since I'm not maintaining this anymore you may want to wait for further review, but I wanted to give you some feedback

src/c-writer.cc

test/run-spec-wasm2c.py

wasm2c/README.md

wasm2c/examples/fac/fac.wat

keithw · 2022-04-05T05:36:58Z

@binji Thank you muchly for the reviews! I wish GitHub had a way for us to mark the stacked PRs more clearly -- if possible, could we take these in order (so #1814 [module instancing] would be next, before we get to this one [bulk memory], reference types [#1887], and tolerating partly invalid modules [#1888])? Some of your comments here are things we'll need to fix in #1814 where the relevant commits originate, and then our plan was to rebase the later PRs on top of whatever ultimately lands.

binji · 2022-04-15T20:46:30Z

Sorry about reviewing out of order, I should have looked more closely at the description!

sbc100 · 2022-04-28T15:49:39Z

Does this change really depend on instancing ( #1814)? I wonder if it can land first?

keithw · 2022-04-28T17:59:16Z

Does this change really depend on instancing ( #1814)? I wonder if it can land first?

It's possible, but it would be a bit of work to backport this to a pre-instanced implementation. I guess I thought we had coalesced on this ordering when we talked at #1853 (comment).

The major "instance-related" design decisions/consequences here are:

as before, we store each segment as a static const array at file scope (belonging to the "module" itself), but now each module instance has a boolean (in a bitfield) to track whether the corresponding segment has been dropped. (A drop instruction doesn't actually free memory here; even if all extent instances have dropped the segment, the caller might later create another instance of the same module...)
before bulk memory, it was possible to use element segments only at "wasm2c-time", to write the code that initializes the table as part of init_table, and then not actually store the element segment itself. But after bulk memory, the element segment has to be stored so that it can be used at runtime in a table.init. To do that, we have to be a bit careful in choosing the representation of the init expression, because at runtime the members of a funcref-type table are function instances (which close over the originating module), but when writing down the element segment itself (before any module has been instantiated with its imports), you don't know what the originating module will be. So, we represent the elem segment init expr as a local type index, a funcref, and an offset within the module instance structure to the originating module pointer. At runtime, the implementation of table_init converts this to a global type index, the funcref, and the actual pointer to the originating module (looked up from the module instance structure).

sbc100 · 2022-04-28T18:01:37Z

Does this change really depend on instancing ( #1814)? I wonder if it can land first?

It's possible, but it would be a bit of work to backport this to a pre-instanced implementation. I guess I thought we had coalesced on this ordering when we talked at #1853 (comment).

The major "instance-related" design decisions/consequences here are:

as before, we store each segment as a static const array at file scope (belonging to the "module" itself), but now each module instance has a boolean (in a bitfield) to track whether the corresponding segment has been dropped. (A drop instruction doesn't actually free memory here; even if all extent instances have dropped the segment, the caller might later create another instance of the same module...)

before bulk memory, it was possible to use element segments only at "wasm2c-time", to write the code that initializes the table as part of init_table, and then not actually store the element segment itself. But after bulk memory, the element segment has to be stored so that it can be used at runtime in a table.init. To do that, we have to be a bit careful in choosing the representation of the init expression, because at runtime the members of a funcref-type table are function instances (which close over the originating module), but when writing down the element segment itself (before any module has been instantiated with its imports), you don't know what the originating module will be. So, we represent the elem segment init expr as a local type index, a funcref, and an offset within the module instance structure to the originating module pointer. At runtime, the implementation of table_init converts this to a global type index, the funcref, and the actual pointer to the originating module (looked up from the module instance structure).

No problem. If its not a simple rebase we can keep the sequence as is

The upgrade broke the Firefox build. wasm2c can't handle bulk memory ops yet. https://bugzilla.mozilla.org/show_bug.cgi?id=1773200 WebAssembly/wabt#1877 git-svn-id: file:///srv/repos/svn-community/svn@1230052 9fca08f4-af9d-4005-b8df-a31f2cc04f65

keithw · 2022-09-20T05:51:34Z

@sbc100 Do you want to take a look at this one? (It will be helpful for updating the testsuite as more of the spec tests have started to require bulk-memory.)

sbc100

Nice!

lgtm with comments

sbc100 · 2022-09-20T07:22:42Z

scripts/gen-wasm2c-templates.cmake

@@ -1,4 +1,5 @@
 # https://stackoverflow.com/a/47801116
 file(READ ${in} content)
-set(content "const char* ${symbol} = R\"w2c_template(${content})w2c_template\";")
+string(REGEX REPLACE "(.[^\n]*\n)" "R\"w2c_template(\\1)w2c_template\"\n" content "${content}")
+set(content "const char* ${symbol} = ${content};")


Why this change?

MSVC has a limit on the length of string literals that was getting exceeded (I think because wasm2c.declarations.c is now longer than 16KiB). This switches it to being one string literal per line of source.

sbc100 · 2022-09-20T07:30:08Z

src/template/wasm2c.declarations.c

@@ -444,4 +444,85 @@ static float wasm_sqrtf(float x) {
  return sqrtf(x);
 }

+static inline void memory_fill(wasm_rt_memory_t* mem, u32 d, u32 val, u32 n) {


Would it make sense to somehow only emit this code when bulk memory is enabled? Perhaps we could define _WASM2C_HAS_BULK_MEMORY and make this conditional? I guess it doesn't make much difference? Maybe tiny modules would compile marginally faster?

Maybe not worth it?

We can do this if you want, but given that bulk memory is enabled by default (and has been for almost a year), it seems like a niche case... and it's not like these are particularly complicated functions to compile. (Some of the math functions are arguably more complicated and we emit those unconditionally...)

wasm2c/examples/fac/fac.c

sbc100 · 2022-09-20T07:31:22Z

wasm2c/examples/fac/fac.c

+}
+
+static void init_elem_instances(Z_fac_instance_t *instance) {
+}


It it worth eliding all these function when they are empty (to make the output more readable?)

Sounds good, done in #1999 which hopefully we can do as a follow-on to this one.

sbc100 · 2022-09-20T07:33:26Z

src/c-writer.cc

+      Write("bool ", "data_segment_dropped_", GetGlobalName(data_segment->name),
+            " : 1;", Newline());
+    }
+  }


I wonder if we should have some more examples of wasm2c output actual checked in so that when reviewing these changes we can see the effect they have on the output? Not sure what the best way to do that is.

I also like this idea but not sure how far to take it... some options are probably:

check in rot13.c like we do fac.c?

check in a few representative outputs of running wasm2c on the spec testsuite?

check in every single output of running wasm2c on the spec testsuite?

How about a few hand-crafted tests under test/wasm2c that include the input wat and the output c code in the same same file.

Did a first stab at this in #2006.

src/c-writer.cc

…VC limit

Co-authored-by: Yuhan Deng <yhdeng@stanford.edu>

keithw requested a review from sbc100 March 31, 2022 21:40

keithw force-pushed the bulk-memory branch from 56a5fcf to e91ab65 Compare March 31, 2022 21:45

keithw changed the title ~~wasm2c: implement the bulk-memory operations proposal~~ wasm2c: implement the bulk memory operations proposal Mar 31, 2022

keithw force-pushed the bulk-memory branch 2 times, most recently from dae6e37 to 7c8b9a5 Compare April 4, 2022 10:43

This was referenced Apr 4, 2022

wasm2c: implement the reference-types proposal #1887

Merged

wasm2c: support final (non-SIMD) spec tests by adding an unsafe "ignore validation failure" flag #1888

Closed

keithw requested a review from kripken April 4, 2022 17:17

keithw force-pushed the bulk-memory branch from 7c8b9a5 to 9f7723e Compare April 5, 2022 04:09

binji approved these changes Apr 5, 2022

View reviewed changes

keithw added a commit to fix-project/wabt that referenced this pull request Apr 6, 2022

Fixes per WebAssembly#1877 (review)

c0ae781

keithw force-pushed the bulk-memory branch 2 times, most recently from fdec286 to f5d4deb Compare April 6, 2022 11:34

This was referenced Apr 6, 2022

wasm2c: add support for module instancing #1814

Merged

Remove signature mangling from wasm2c output #1896

Merged

keithw force-pushed the bulk-memory branch 3 times, most recently from 85e60d0 to f36fb7c Compare April 28, 2022 05:45

keithw force-pushed the bulk-memory branch from f36fb7c to 70a9351 Compare May 15, 2022 05:58

keithw mentioned this pull request Jun 1, 2022

wasm2c: implement the exception-handling proposal #1930

Merged

This was referenced Jun 10, 2022

Bulk memory problem with wat2wasm + wasm2c #1934

Closed

0000075: error: invalid section code: 12 #1935

Closed

keithw mentioned this pull request Jun 30, 2022

wasm2c: run tests with -O2 on non-Windows #1939

Merged

keithw force-pushed the bulk-memory branch from 9e7b774 to 9107eac Compare July 15, 2022 06:40

keithw mentioned this pull request Aug 11, 2022

000002c: error: invalid section code: 12 #1955

Closed

keithw force-pushed the bulk-memory branch 4 times, most recently from e160874 to 13642fc Compare August 16, 2022 01:15

keithw force-pushed the bulk-memory branch from 13642fc to d8c492f Compare August 24, 2022 15:13

keithw force-pushed the bulk-memory branch 2 times, most recently from de1e4dd to 1793a5d Compare September 16, 2022 16:58

keithw mentioned this pull request Sep 20, 2022

Implement Relaxed SIMD proposal #1994

Merged

sbc100 reviewed Sep 20, 2022

View reviewed changes

sbc100 approved these changes Sep 20, 2022

View reviewed changes

yhdengh and others added 8 commits September 20, 2022 21:40

Fix register command for named modules

d0c5fc2

gen-wasm2c-templates.cmake: use line-by-line string to work around MS…

9943a8f

…VC limit

wasm2c: implement bulk memory operations

3493b41

Add multi-memory load and store tests

dee6c04

Restore current version of elem.wast test

3c158db

Add bulk-memory tests

f95f8a0

Update README.md

af0dfd6

c-writer.cc: add newline after init_globals()

a849c2c

keithw force-pushed the bulk-memory branch from 5263d92 to a849c2c Compare September 21, 2022 05:04

keithw mentioned this pull request Sep 21, 2022

wasm2c: eliminate unused initialization functions in output #1999

Merged

c-writer.cc: make conditional treatment of data segments consistent

b716f71

keithw enabled auto-merge (squash) September 21, 2022 05:41

keithw merged commit 608d23e into WebAssembly:main Sep 21, 2022

keithw deleted the bulk-memory branch September 21, 2022 06:00

keithw mentioned this pull request Sep 24, 2022

Add wasm2c output samples to tests #2006

Merged

talg pushed a commit to PLSysSec/wabt that referenced this pull request Sep 26, 2022

wasm2c: implement the bulk memory operations proposal (WebAssembly#1877)

a03de93

Co-authored-by: Yuhan Deng <yhdeng@stanford.edu>

matthias-blume pushed a commit to matthias-blume/wabt that referenced this pull request Dec 16, 2022

wasm2c: implement the bulk memory operations proposal (WebAssembly#1877)

d177456

Co-authored-by: Yuhan Deng <yhdeng@stanford.edu>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm2c: implement the bulk memory operations proposal #1877

wasm2c: implement the bulk memory operations proposal #1877

keithw commented Mar 31, 2022 •

edited

Loading

binji left a comment

keithw commented Apr 5, 2022

binji commented Apr 15, 2022

sbc100 commented Apr 28, 2022

keithw commented Apr 28, 2022

sbc100 commented Apr 28, 2022

keithw commented Sep 20, 2022

sbc100 left a comment

sbc100 Sep 20, 2022

keithw Sep 20, 2022

sbc100 Sep 20, 2022

keithw Sep 20, 2022

sbc100 Sep 20, 2022

keithw Sep 21, 2022

sbc100 Sep 20, 2022

keithw Sep 21, 2022 •

edited

Loading

sbc100 Sep 21, 2022

keithw Sep 24, 2022

wasm2c: implement the bulk memory operations proposal #1877

wasm2c: implement the bulk memory operations proposal #1877

Conversation

keithw commented Mar 31, 2022 • edited Loading

binji left a comment

Choose a reason for hiding this comment

keithw commented Apr 5, 2022

binji commented Apr 15, 2022

sbc100 commented Apr 28, 2022

keithw commented Apr 28, 2022

sbc100 commented Apr 28, 2022

keithw commented Sep 20, 2022

sbc100 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithw Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithw commented Mar 31, 2022 •

edited

Loading

keithw Sep 21, 2022 •

edited

Loading