chore: update Binaryen #2927
chore: update Binaryen #2927
Conversation
|
|
| ) | ||
| (func $start:function-call~anonymous|2 (param $0 i32) (param $1 i32) (result i32) | ||
| local.get $0 | ||
| local.get $1 | ||
| i32.add | ||
| ) | ||
| (func $start:function-call~fn2|4 (param $0 i32) (result i32) | ||
| i32.const 1 | ||
| local.get $0 |
There was a problem hiding this comment.
Hmm, this definitely got worse.
There was a problem hiding this comment.
I believe these two cases are caused by the same issue (the dae/DeadArgumentElimination pass not doing its job? I'm not fully sure).
I did a bisection to find the Binaryen commit that's responsible. (I tested on the call-optional WAT file in S-expr format, but the issue applies here as well.) It looks like WebAssembly/binaryen#7135 is the cause, as that PR treats any funcref as being possibly leaked to the outside world...
How should we proceed? I don't think marking the module as closed-world is accurate...
There was a problem hiding this comment.
@MaxGraey Should we just eat the cost and merge this PR? We could also mark the module as closed world if exported functions don't return funcrefs and tables aren't imported/exported...what do you think?
There was a problem hiding this comment.
We could also mark the module as closed world if exported functions don't return funcrefs and tables aren't imported/exported...what do you think?
That would be ideal because it's a quite common scenario.
There was a problem hiding this comment.
@MaxGraey I made that change now!
(Now that I think about it, AS currently never exports arrow functions/etc. as funcrefs, but as boxed objects instead...regardless, the change accounts for table imports/exports as well as users specifying ref_func exports, or exported functions returning ref_func.)
Some new features are being added, although only relaxed SIMD is in a usable state. stringref has been almost obliterated, and only functionality compatible with JS string builtins is being kept. Binaryen also seems to be generating more nops and blocks than usual. Also, due to WebAssembly/binaryen#7135, dead arguments aren't being eliminated when a function reference is made (ref.func), which affects the optimized output of call-optional, function-call, and other tests.
tests/compiler/comma.debug.wat
Outdated
| @@ -152,6 +152,7 @@ | |||
| global.get $comma/a | |||
| i32.lt_s | |||
| if | |||
| nop | |||
There was a problem hiding this comment.
Hmm. Do you have any assumption why the nops started showing up?
There was a problem hiding this comment.
@MaxGraey The function that wrote WAT (StackIR) was passed a true argument to optimize the StackIR, but this was changed to a global option. Setting this global option removes these nops.
Due to WebAssembly/binaryen#7135, any function reference is treated as potentially leaked to the outside world, unless the module is marked as closed-world. This prevents DAE from taking place. To fix this, mark the module as closed-world when tables are neither imported nor exported and when exports do not return/contain funcrefs.
In WebAssembly/binaryen#6568, StackIR was changed from a set of passes to a global option. This meant that the `true` argument passed into _BinaryenModuleAllocateAndWriteStackIR (to optimize the StackIR) no longer had an effect. This commit restores that and also runs the optimizations under the same {optimize,shrink}Levels as wasm-opt.
Changes proposed in this pull request:
⯈ Updating Binaryen to v123