WIP: Transpile sign extension proposal

[maurobringolf]

Here is what I've been working on:

• New package that transpiles sign extension instructions back to functions with the (hopefully) same behavior.

• The functions replacing the instructions are written in assemblyScript, compiled to wasm, parsed by wasm-parser and then injected into a given ast.

• Only the necessary functions are inserted

The package is currently written as ast->ast transform and the tests are setup as text->text for readability. We can add another API using wasm-edit for binaries directly. I would like to have a test setup for functional correctness though and haven't really figured out how to do that yet.

[ColinEberhardt]

Nice idea :-)

The use of Assembly Script to generated the function that polyfills does feel like overkill. The generated WAT is really quite simple:

  (func $main/i32_extend16_s (export "i32_extend16_s") (type $t2) (param $p0 i32) (result i32)
    get_local $p0
    i32.const 32768
    i32.and
    if $I0 (result i32)
      get_local $p0
      i32.const -32768
      i32.or
    else
      get_local $p0
      i32.const 32767
      i32.and
    end
    return)

I'd just use the above wat directly, parsing it into AST, then 'injecting' it as part of your polyfill transform.

[maurobringolf]

Thanks for the feedback! It might very well be overkill and caused some extra work in the build process. It simply represents the way I arrived at these (first in C, Sven suggested migrating to AssemblyScript). There might be one advantage though: I am looking for a way to test the functional correctness of the polyfill functions. One possibility would be to test the AssemblyScript source. Do you have an alternative suggestion for this kind of testing?

We could use the interpreter, but its probably not ready yet? Also much more errorprone I imagine

[ColinEberhardt]

I am looking for a way to test the functional correctness of the polyfill functions. One possibility would be to test the AssemblyScript source. Do you have an alternative suggestion for this kind of testing?

If you write these functions in WAT, I'd just unit test that via JS - e.g. unit test the $main/i32_extend16_s function I added in the comment above

[xtuc]

do we need to commit the AST generated by the polyfill compilation?

[maurobringolf]

I was not sure how releases / packaging works. The proposal package directly loads the json ast to avoid unnecessary parsing. We could load and parse from WAT as suggested by Colin as an alternative

[xtuc]

We could use the interpreter, but its probably not ready yet? Also much more errorprone I imagine

How? It's not a C compiler.

Assembly Script to generated the function that polyfills does feel like overkill

I think it will be more maintainable

[maurobringolf]

@xtuc Its not a C compiler yet. Just kidding 😄 I meant that the interpreter could be used to run the polyfill wasm function to test functional correctness.

[ColinEberhardt]

Assembly Script to generated the function that polyfills does feel like overkill

I think it will be more maintainable

For complex polyfills I'd agree, but in this case it's a few lines of WAT. I like simple :-)

Also, how stable is AssemblyScript? If you encounter an error, can you be sure it is in your code? Or is it the AssemblyScript compiler? How optimised is the output?

Anyhow, this is not a strong objection, just my thoughts!

[xtuc]

Yeah I agree @ColinEberhardt. What do you think @maurobringolf?

Our interpreter is definitely not ready.

[maurobringolf]

Less things to maintain is fine with me 👍 It was a fun experiment. Would you make the wat->ast translation build-time or runtime?

[xtuc]

We could do it at publish-time but runtime seems easier for now.

[ColinEberhardt]

Yeah, I’d go runtime for simplicity. It’s a pretty simple transform and transforms aren’t really that performance critical

[maurobringolf]

Thanks for the feedback, I prefer text format as source for the polyfills now too. I also realized that the other approach would not work in the long run anyways: As soon as a feature becomes standard it wouldnt make sense to use a compiler anymore since it might output code that relies on the feature (instructions in this case) we're trying to polyfill in the first place.

• Now .wast polyfills are loaded from source files and parsed with wast-parser
• I added functional tests for both i32 polyfills by converting the wast source to wasm with wat2wasm from wabt and then running it as module in node directly.
• Not sure what to do with i64

[xtuc]

Maybe we should change that to support two or more bytes? Atomic operations also have an additional byte.

[maurobringolf]

Are you referring to the threads proposal? i.e. https://webassembly.github.io/threads/binary/instructions.html#atomic-memory-instructions. Then it shouldn't affect this proposal but something to look out for in the future I think?

[xtuc]

I already added a check for the atomic operations prefix byte. I just saying that we could move the logic here by matching an array of bytes?

[xtuc]

I'm not sure this test is very useful

[maurobringolf]

Ah this was my test to check that the test setup works before having the transform. I dont think it is useless though, empty function body might trigger an edge case somewhere. But we might also remove it

[xtuc]

Not sure what to do with i64

Sorry, what do you mean

[maurobringolf]

@xtuc There is no functional test for i64 polyfills since we cannot call functions directly from JS.

[xtuc]

You can deactivate the check: https://github.com/xtuc/webassemblyjs/blob/master/packages/repl/src/index.js#L299-L307

We are testing i64 in the spectest via our REPL.