[Wasm2JS] More optimal JS codegen for some special cases

[MaxGraey]

This integer binary operations can be simplified in JS domain:
0 - x | 0 -> -x | 0
x ^ -1 | 0 -> ~x | 0
x * C_safe -> x * C_safe | 0 where C_safe is constant in [-0x1FFFFF, 0x1FFFFF] range
All this Closure Compiler can't optimize even in Advance mode.

Additionally skip redundant integer binary coercion for bitwise operations and coercion for returns if it's binary or unary expression which already should be coerced.

Partially fix #2951

[MaxGraey]

Fuzzed: ITERATION: 16990

[kripken]

In general it sounds good to optimize JS output, but how much do these things help? Is there a benchmark or real world use case you are measuring on?

[MaxGraey]

In general it sounds good to optimize JS output, but how much do these things help? Is there a benchmark or real world use case you are measuring on?

I don't think it could significant optimizing except Math.imul rule (which btw was TODO) but it could reduce final size and speedup minimization via Closure Compiler

[kripken]

Makes sense. I'm asking just because it takes time to write and review these changes, so I'm trying to figure out how much priority to put on it, at least for myself.

@juj - would these type of optimizations be useful for your projects?

[juj]

@juj - would these type of optimizations be useful for your projects?

Yes, (theoretically) they would be useful and very much appreciated. It reads like this PR might be able to affect emscripten-core/emscripten#13362 ? (or if not, that is closely related at least?)

However in practice, we unfortunately had to conclude a bit more than a year ago that current Wasm2JS output is too large, takes too long to build (the necessary Closure step takes ages) and generated builds are an order of magnitude slower to run on the target devices/webviews compared to what fastcomp produced, that we had to abandon Wasm2JS related business cases altogether. Both classic Unity and Tiny Unity are now Wasm-only - producing Wasm2JS builds for debugging is unfortunately also no longer feasible.

I did try to figure out some of the suboptimalities from the generated wasm2js code back then (some old reports from that attempt I could now find).

There were some attempts to do dual builds with both fastcomp and wasm backends, but I think in the end those attempts got blocked due to fastcomp compiler path diverging behind from wasm backend compiler.

If there were a way to improve Wasm2JS build times, runtime execution speed and output sizes each by an order of magnitude back to the fastcomp asm.js levels, then it would be interesting - but at this stage I presume that would be too much diminishing-returns effort for anyone to undertake.

Maybe the only practical use for me is that I do regularly report WebGPU spec&implementation bugs from https://github.com/juj/wasm_webgpu repository. That contains very small compiled test cases. When reporting such bugs, past experience shows that people are put off by bug reports that contain any .wasm code and tend to reject them asking for a small test case (even though the .wasm test case might already be a trivially small MINIMAL_RUNTIME based test case with no dynamic dead code). Because of that, I generally report all bugs using Wasm2JS SINGLE_FILE build mode, in order to get a single as small as possible .html file for a bug report.

Having a better Wasm2JS codegen there would help in practice people who read and process those bugs.

[MaxGraey]

Yes, (theoretically) they would be useful and very much appreciated. It reads like this PR might be able to affect emscripten-core/emscripten#13362 ? (or if not, that is closely related at least?)

No, x + 0.0 is unsafe in term of wasm. It could be eliminated by turning on fastMath mode in Binaryen. However it will remove by Closure Compiler, so I guess I could add reduction rule for this case as well. But I'm not sure due to -0.0 + 0.0 will return -0.0 instead 0.0 in this case.

If there were a way to improve Wasm2JS build times, runtime execution speed and output sizes each by an order of magnitude back to the fastcomp asm.js levels, then it would be interesting - but at this stage I presume that would be too much effort for anyone to undertake.

I also interested in speedup JS builds. Please look at this PR as well: #4055
It gives a significant speedup for 64-bit division when the divisor is a constant for AsmJS code.

[MaxGraey]

@juj If you have any other suggestions for reducing the size or speeding up Wasm2JS, I would be interested to hear about it.

[kripken]

Interesting @juj , thanks for the update.

Some of the build time issues might be improved, but an order of magnitude slowdown to run suggest that your target devices were using actual asm.js optimizations (either literally doing validation, or indirectly) - that would be challenging to work on. Wasm is basically the solution for such optimization cliff issues... so makes sense to just go and do wasm everywhere I guess.

Personally I still use wasm2js builds a lot for debugging though.

@MaxGraey Given all that, I think JS-only optimizations like this might be low-priority, if there aren't users that really need them. Without such users, there are a lot of things that we can do to make wasm builds better - I'd prefer to focus on that.

[MaxGraey]

@MaxGraey Given all that, I think JS-only optimizations like this might be low-priority, if there aren't users that really need them. Without such users, there are a lot of things that we can do to make wasm builds better - I'd prefer to focus on that.

I understand. But by the way, many continue to use asmjs builds. AssemblyScript, for example, uses binaryen.js for exactly this purpose. It allows you to get a better stack trace if something goes wrong. Also, some commercial products still maintain UC Browser + IE11 and have to use wasm2js for that. For example: https://engineering.q42.nl/webassembly

[MaxGraey]

I know also Rustwasm community uses wasm2js in some cases. cc @alexcrichton

[kripken]

@MaxGraey Yeah, if there are use cases that really need to optimize every bit of size and speed, I agree we should do this. I'd just like to make sure those exist first.

(Many of the use cases, like getting better stack traces, are useful for debugging but don't require maximum speed. My own debugging usage of wasm2js is like that.)

[MaxGraey]

Another problem is that we would be happy to use binaryen.wasm, but we have some difficulty wrapping custom binaryen's wrapper into ESM modules. Daniel tried it in this PR: #3304 but it seems to have stalled.

[dcodeIO]

Iirc from the PR, the challenge there is a little like a Hen and Egg problem, in that we have two choices that have their own tradeoffs:

1. Rely on what Emscripten produces with MODULARIZE/ES6, but then we get an unnecessary factory / breaking change. Conceptionally, Binaryen(.js/.wasm) would be best served with what used to be MODULARIZE_INSTANCE. Changes to Emscripten would be the way to go here.
2. Use custom pre-js glue code for Binaryen, then re-implementing what Emscripten would generate if there still was MODULARIZE_INSTANCE (incl. ready promise etc.) and ES6 (since we cannot re-import/export from Emscripten-generated glue in the same file). I tried that in the PR, but didn't resonate so well.

Generally, Binaryen(.js/.wasm) is a little special in that it is really a single instance so a factory as anticipated by Emscripten is overkill / inconvenient for it. It wouldn't look / behave like a normal ES/Node module with what's currently available.

And Binaryen.wasm is connected, in that the artifact becomes async by means of instantiate (while .js is sync), so addressing both at once would be good to prevent unnecessary work / workarounds when just aiming at one.

The bottom line is that I am hesitant to make the required changes to Emscripten, as these would be breaking (for the better in the long run, though) I think, and that attempting the alternative in 2. led to quite a bit of bike shedding / misunderstanding that I didn't know how to continue with it.

[kripken]

Interesting. I agree focusing on binaryen.wasm is the right path. Sadly I am maybe not the best person to help with the ES6 module stuff since I really don't know all the details, but I'll try...

About option 1, if emscripten emits an ES6 module that produces a factory, can we call that binaryen.factory.js and then write a small wrapper here in Binaryen called binaryen.js that just creates an instance of it? And all uses would go through there. That sounds simpler than option 2 but maybe I'm missing something.

(cc @RReverser for ES6 modules stuff)

[dcodeIO]

If we want to produce a single .js file, I think we would have to postprocess binaryen.factory.js with some magic since it has conflicting ES6 (default) exports then that we can only amend statically (unlike in ES5 where we can dynamically re-assign things in custom wrapper code). If we instead generated ES5 to put into an ES6 wrapper, we'd be missing import.meta to discover the .wasm file once we turn the wrapped module into an ES6 module, being left with ES6-incompatible discovery. Perhaps if we can update the Emscripten code used to obtain the .wasm location to work with both ES5 and ES6 it would become possible, but not sure if this can be done.

[dcodeIO]

Concretely, what we could perhaps try is to generate the binaryen.factory.js as suggested, and run an ES6-aware bundler afterwards to combine it with the instance wrapper into a single .js file. We could do that in the Binaryen repo itself, or outsource the bundling part to the Binaryen.js buildbot. That would probably have to export the instance as a single default export, though.

[kripken]

If a JS bundler can do this, that sounds promising to me! In that case I don't have a preference for which repo this is done in.

A single default export from the combined JS sounds good to me, although I am not close enough to current conventions in the JS world to have a strong opinion.

[dcodeIO]

Cool! Concrete next steps could be to switch over to generate an ES module and mark it as a breaking change, so Max and I can then test-drive a mechanism on top of the buildbot, and if you like (probably will add an npm dependency for a bundler) pull it upstream? Does this sound good? :)

[kripken]

Sorry for missing this update @dcodeIO - sounds good!