WebAssembly / spec

WebAssembly specification, reference interpreter, and test suite.
https://webassembly.github.io/spec/
Other
3.11k stars 439 forks source link

Make "large" compatible with wasm2kotlin #1630

Closed SoniEx2 closed 1 year ago

SoniEx2 commented 1 year ago

Closes #1619

sunfishcode commented 1 year ago

The existing test appears to contain 25340 entries, which is large, but not unreasonably large in contexts like binary decoding or lookup tables.

One of the strengths of Wasm is that one can run the same Wasm module in many places. Every time an implementation introduces a limitation based on the size of something, it compromises that strength. Sometimes this is practically unavoidable. But here, obliging an implementation to split large jump tables into smaller jump tables by using eg. binary search using comparisons and branches does not seem unreasonable.

sunfishcode commented 1 year ago

@rossberg Is there a reason why my concern here seems to have been unilaterally dismissed without any explanation?

rossberg commented 1 year ago

@sunfishcode, sorry, I merged before seeing your comment (some github hickup).

But I think this is fine, because (a) the test still is pretty large, and (b) it is not a negative test -- nothing prevents larger examples from passing.

We generally expect different embedding environments to impose different limits. There is something to be said about broader standards for minimum limits, but the core test suite is not the right place to enforce this. I don't think it has a mandate for that, as long as such limits are not actually stated in the core spec (which they intentionally aren't). Rather, the test suite should cater to a diverse range of embedders.

SoniEx2 commented 1 year ago

@sunfishcode for context, even something like blastem, whose opcodes are 16-bit, doesn't use anywhere near that many table entries.