Switching to use upstream LLVM backend by default

[dschuff]

The upstream LLVM WebAssembly backend (along with its emscripten integration) is approaching parity with the fastcomp+asm2wasm pipeline, and we'd like to put in some extra effort to push it over the finish line, so that we can set it as the default for emscripten. I'd like to use this issue for discussion and maybe a tracking bug (although github's support for that use case isn't great; so maybe a project board instead?).

There are a few categories of stuff to think about:

1. Bugs and missing LLVM/backend functionality (e.g. miscompiles, IR we don't lower yet)
2. Missing emscripten features compared to fastcomp + asm
3. Performance (compile time and generated code quality)
4. ABI or any behavior changes that we don't necessarily consider a bug but need some sort of special consideration.

So what's the current status?

1. There are a small number of known bugs (e.g. some tuning for the FixFunctionBitcasts pass, and the problem of optimizers moving trapping conversions outside of guard conditions) and known missing features. But the emscripten test suite works, and with a couple of hacks, large programs such as BananaBread work.
2. IIRC there are a couple of these but no major ones. I think @jgravelle-google knows more.
3. There has been some small investigation into this (but no large benchmarks that I know of?), but with @kripken's recent work in the binaryen optimizer I believe code performance is very close. There will be some compile time regression in the short term (until we get wasm-level linking), but hopefully not pathological.
4. We know that the LLVM triple and its associated ABI is different, and we've talked about wanting to have asm.js and wasm triples match.

What do we need to do

1. Fix the known bugs, obviously. @aheejin is taking another look at our existing lists of known failures to make sure we understand the causes. Because the new backend doesn't have the realworld hardening that fastcomp does, I would feel better if we had even a bit more testing. It would be nice to get the LLVM test suite running, even if in parallel to the other work. @sbc100 has started this.
2. Do a bit more testing on larger benchmarks that we care about.
3. I would like to merge the triples/ABIs in one way or another. We had talked about changing fastcomp to match the upstream backend in a lot of the cases (although I do want to get rid of 128-bit long doubles, it's proven already to be just not worth the upkeep cost). Although another option might be to think about using wasm2asm going forward.

Thoughts?

[kripken]

Nice summary, good to see this making progress.

But the emscripten test suite works, and with a couple of hacks, large programs such as BananaBread work.

It's worrying to me that the test suite passes but BananaBread fails, please let's add a test for that issue in the emscripten test suite when we figure it out.

I believe code performance is very close

At least on the emscripten benchmark suite, speed looks ok aside from what to do with the lsr pass (need feedback there).

Code size is an issue, we need to investigate that more carefully. I saw a significant difference (5-10% I think I remember?) but it was a while ago so probably out of date.

There will be some compile time regression in the short term (until we get wasm-level linking), but hopefully not pathological.

I lean towards waiting for wasm-level linking for switching the default, since I think the compile time difference was large last I checked (2x or so). But if it's small of course (removing lsr might help) that's not an issue.

[juj]

This sounds exciting!

@dschuff : I have access to a number of project repositories that I can try to compile over with the new backend, ranging from small tests to large commercial projects. Your description sounds like it's a good moment for me to start trying out the new backend on these.

Before we default to the new backend, we need to make sure to be able to compile existing projects ok, or if there are some items we need to not support, we should build a migration guide and work on a deprecation path. We should definitely ship the new backend in Emscripten SDK for one version at least with it being available but optional/default off, so that we can also gather feedback from people at emscripten-discuss.

Can you document if there are any current gotchas with getting going with building with it? I presume the latest code should be available at https://github.com/llvm-mirror master branch and no need to root to the SVN repository? Are there other knobs to set except the environment variable EMCC_WASM_BACKEND=1?

[GGAlanSmithee]

Hope you don't mind me asking, bit will there be any significant change in disk space requirements if the backend is changed? Currently it seems like the fastcomp one requires ~13GB in total which is kind of a lot (using a virtual workspace). Thanks

[dschuff]

@GGAlanSmithee I wouldn't expect there to be a big change. We'll still need LLVM and clang, it will just be the upstream versions instead of fastcomp. I'd guess it will be a bit larger just because it's a newer version of the ever-growing LLVM project. We'll also eventually need lld, although that's not a huge codebase.

[dschuff]

@juj yes, I think now is a great time to start doing that. And as long as nobody minds building/downloading LLVM twice :D shipping an emsdk with support for both sounds good. Manually setting EMCC_WASM_BACKEND actually shouldn't be necessary because emcc queries the llc pointed to by LLVM_ROOT to see what targets it supports (and if it has the wasm backend but not fastcomp, it should just work).

@kripken I'd lean toward not requiring wasm-level linking, because

1. that's just one more thing to switch at once
2. it will delay the release of the wasm backend by an unknown amount, and all new feature work we do until then has to be done twice.
   But you are right that we should more carefully evaluate the compile time costs; it's also on my list to hammer on the wasm backend's fast-isel a bit to make sure it's not bailing out too often.

[GGAlanSmithee]

@dschuff thanks for the explanation! 👍

[stale]

This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

[jakirkham]

Has this been resolved?

[sbc100]

Ha, good call. I guess its not quite the default yet so lets leave this open.