search

WebAssembly / memory64

Memory with 64-bit indexes
Other
194 stars 21 forks source link

WebAssembly.Memory "index: u32/u64" constructor parameter #39

Closed dakenf closed 11 months ago

dakenf commented 11 months ago

Currently spec does not allow creating 64 bit memory from JS code, so there is no way to create a shared memory instance with more than 65,536 pages.

I propose to add an optional parameter that would create a 64 bit memory instance without those limits.

sbc100 commented 11 months ago

Sounds like index: "u64" is what folks suggested in #24. Any reason not to go with that?

dakenf commented 11 months ago

No, it's even better than my proposal. Will update the PR

dakenf commented 11 months ago

I've fixed V8 to work with 64bit memory and threads. Except changes in WebAssembly.Memory there was also an issue with compiler that prevented passing i64.const to memory.fill and memory.init. I think I will clean up the code and make a PR in next few days. Can we get this reviewed and merged? I'm glad to make any changes to get it done

backes commented 11 months ago

It looks like your CL to add Memory index type support in the JS API to V8 is your first contribution to V8 (https://crrev.com/c/4742982). Or did you use another mail address before?

Anyway, we should land this PR before adding support in V8. Can you also add spec tests for the new parameter please?

dakenf commented 11 months ago

It was my first contribution, will add the tests today

dakenf commented 11 months ago

I've added JS tests. There were memory64 tests in test/core/bulk64._wast but they are disabled because the interpreter does not support memory.copy, fill and init operations. And as it's written in ocaml, i don't think i can add these op support in a reasonable time

dakenf commented 11 months ago

I went with the easy way, checked presence of type() method and updated assert_Memory to check the index value

sbc100 commented 10 months ago

Oh wait, I think we should have gone with i64/i32 here. This would be consistent with the JS types proposal (see https://github.com/WebAssembly/js-types/blob/main/proposals/js-types/Overview.md) and also with what spidermonkey apparently implement: https://github.com/WebAssembly/memory64/issues/24#issuecomment-1274397516

sbc100 commented 10 months ago

Sorry to flip flop on this.. I know I suggested 'u64' back in https://github.com/WebAssembly/memory64/pull/39#issuecomment-1652059727 but I think I was wrong.

dakenf commented 10 months ago

Yeah, why not. I think i'm the only one who's using it right now so it won't hurt anyone. Will make a new changeset to V8