wabt icon indicating copy to clipboard operation
wabt copied to clipboard

wasm2c: implement the bulk memory operations proposal

Open keithw opened this issue 3 years ago • 5 comments
trafficstars

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

Support bulk memory operations in wasm2c (the next step in the checklist at https://github.com/WebAssembly/wabt/pull/1853#issuecomment-1062094906).

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 #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 #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.)

keithw avatar Mar 31 '22 21:03 keithw

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

keithw avatar Apr 05 '22 05:04 keithw

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

binji avatar Apr 15 '22 20:04 binji

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

sbc100 avatar Apr 28 '22 15:04 sbc100

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 https://github.com/WebAssembly/wabt/pull/1853#issuecomment-1062094906.

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).

keithw avatar Apr 28 '22 17:04 keithw

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

sbc100 avatar Apr 28 '22 18:04 sbc100

@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.)

keithw avatar Sep 20 '22 05:09 keithw