search

WebAssembly / memory64

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

New spec tests #8

Closed aardappel closed 3 years ago

aardappel commented 3 years ago

These are copied from the ones initially added to Binaryen

sbc100 commented 3 years ago

So these tests are verbatim what was already reviewed by @binji when they landed in binaryen?

Does the spec interpreter have any a support for these days? If not I guess that means that this change means that the test suite will be broken in this repo once this lands? Is there precedent for added tests before the interpreter can run time?

sbc100 commented 3 years ago

I am correct in thinking that each of these tests has an existing xxx.wast that the xxx64.wast is based on? How closely to these match? Is it worth adding a comment to the top of each to about keeping them in sync with the original 32 bit versions where changes are made? Is that even a goal that makes sense?

aardappel commented 3 years ago

It doesn't look like @binji approved them in https://github.com/WebAssembly/binaryen/pull/3130

Still, these are copies modified from similar tests for wasm32, and they are now used both in Binaryen and WABT to test wasm64. They are however significantly different enough that keeping them in sync is not going to be easy. Not all tests in these files matter for both targets, as they simply test if memory can be accessed correctly at all, the pointer size used does not matter in most cases. So to some extend these tests are overkill, but I didn't know of a better way to guarantee good coverage of this new feature.

I discussed with Ben and spec interpreter support can come later.

I will land a WABT change to make these test pass once this merges.