Skip to content

Support for separate compilation with LLVM backend #6830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sbc100 opened this issue Jul 9, 2018 · 3 comments
Closed

Support for separate compilation with LLVM backend #6830

sbc100 opened this issue Jul 9, 2018 · 3 comments

Comments

@sbc100
Copy link
Collaborator

sbc100 commented Jul 9, 2018

We now have support for wasm object files in the upstream LLVM backend. This means we can optionally perform separate compilation. However we also want to be able to preserve the existing LTO support in emscripten.

Currently emscripten always outputs bitcode objects and performs some amount of "LTO" by linking with llvm-link and then running opt, in addition there is an --llvm-lto flag which will trigger additional LTO passes over the bitcode.

In order to continue to enable both bitcode objects and native object we have a new different options to choose from:

  1. Enable wasm object files via a specific new -s WASM_OBJECT_FILES option. By default we still produce bitcode objects.
  2. Build wasm object files by default and disable with -flto compiler line flag which requires bitcode objects.
  3. Re-purpose the existing --llvm-lto flag. When this flag is present build bitcode objects, otherwise default to wasm objects.

(1) is probably least disruptive and might be a good option for the initial implementation. We could later change the default and/or have the default driven by -flto

@jgravelle-google
Copy link
Contributor

I think we'd like to wind up in a world where we build object files by default, but we don't need to land there immediately.
(1) is the least disruptive and most emscripten-y, but probably the weirdest ergonomics for a year from now (emcc -s WASM_OBJECT_FILES=0 for compile, emcc --llvm-lto=1 for link?)
(3) overloads an existing flag. We're already going to be changing what this flag does by virtue of how we do the LTO for wasm-backend (via lld instead of llvm-link, right?), but we'd be changing the default from 0 to 1 here for the interim. This is probably not too bad all told.
(2) I think is a good spot to land, because it makes emcc more clang-like, and is one less flag that's different for cross-platform builds. For the intermediate state, should we use -fno-lto? Or introduce a temporary -s flag.

@kripken
Copy link
Member

kripken commented Jul 9, 2018

First of all, this is great! :)

I don't have strong feelings here either way. Keeping flags consistent with current behavior seems nice for migrating to the wasm backend by default more easily, but on the other hand if we're going to change the meaning of flags later on, that could also be surprising. It might be less work to not change things later again, so I guess I'd have a slight preference for that (option 2).

Regarding LLVM LTO passes, currently we don't run LLVM LTO passes by default because historically they've had bugs, increased code size, and been slow. Those might no longer be the case. Regardless though, I think it's worth supporting 3 modes,

a. wasm object files are linked, can't do LLVM LTO
b. LLVM IR is linked, and we do internalize+dce+globalopts (current asm.js backend default behavior; it's fast, safe, and can dce out most of libc in common cases etc)
c. LLVM IR is linked, and we do full LLVM LTO passes (asm.js backend when --llvm-lto 1)

The middle mode may not be that important in general, but for comparisons to the asm.js backend it could help us in debugging. Maybe in the wasm backend we'd do (a) or (c) by default (that is, the same behavior as native), and optionally --llvm-lto 0 would (assuming inputs are bitcode) do (b)?

@sbc100
Copy link
Collaborator Author

sbc100 commented Feb 8, 2019

I'm gonna close this for now since we do support it. We can open a new issue for making it the default perhaps.

@sbc100 sbc100 closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants