Skip to content
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

How much of llvm/clang do we want to ship in the SDK #74

Closed
sbc100 opened this issue Oct 30, 2019 · 9 comments
Closed

How much of llvm/clang do we want to ship in the SDK #74

sbc100 opened this issue Oct 30, 2019 · 9 comments

Comments

@sbc100
Copy link
Member

sbc100 commented Oct 30, 2019

Right now the SDK consists of two main parts:

  1. the wasi sysroot
  2. a recent build llvm from stable branch

In (2) list of llvm components we install is current long and hardcoded and includes many binaries but also libllvm/libclang/headers that allow for tools to be build with llvm itself.

I'm not sure what logic was used to decide which llvm components to install but I think we would be useful to make a conscious decision: Are we shipping a compiler to end users (e.g. apt-get install clang) or do we also want to allow compiler developers to use the SDK (e.g. apt-get install llvm-dev).

For something like wasi-sdk I think we should really focus on keeping it small that targeted and as such we should just install the minimal set of end user tools that make sense (hopefully llvm has an install target such as "install-user-tools" that encapsulates that for us so we don't need to maintain out large subset of install targets.

If we do want to ship a "fat" SDK when maybe we just want to install everything (.e.g make install)?

My guess is that most people doing compiler development already have their own llvm either in the form an llvm-dev package or a source checkout.. but perhaps I'm wrong.

@sbc100 sbc100 changed the title How much of llvm/clang do we want to skip in the SDK How much of llvm/clang do we want to ship in the SDK Oct 30, 2019
@sbc100
Copy link
Member Author

sbc100 commented Oct 30, 2019

Do we care about the size of the SDK downloads? How much?

@pchickey
Copy link
Collaborator

pchickey commented Oct 30, 2019

This package started out before even llvm-dev packages included support for wasm32-wasi. In those days we used specific commits in the tree, experimental build flags, and even some out-of-tree patches iirc. All of that early work was credit to @yurydelendik.

Aside from providing a recent llvm build, its all configured so you don't have to pass target or sysroot flags if you install it at the right location, which is pretty convenient.

I don't keep a llvm source checkout around because I'd rather just install the wasi-sdk :).

The use cases for this package over at fastly are:

  • test suite for Lucet has a bunch of C that we compile to wasm via wasi-sdk. The Dockerfile in the lucet repo installs this package for use by developers locally, or in CI.
  • same, in test suites for our various internal stuff using lucet. We override some of the environment variables used in the build script to change the path our internal package lives at.

We could swap out the recent llvm build for an llvm-dev package in either of those cases, but then we'd have to manually add the flags and whatnot. But then if we ever needed to vendor a patch in, we'd have to do the legwork to recreate this package.

Right now the deb download is 24M, which isn't too bad. Our internal package that provides every single LLVM tool is something like 400M, which is big enough to start noticing. I'd hope we can keep this under 50M, but I don't have a super strong opinion on that.

@sbc100
Copy link
Member Author

sbc100 commented Oct 30, 2019

I think I've found the solution for installing just the parts we want without a huge whitelist: LLVM_INSTALL_TOOLCHAIN_ONLY.

This should give us exactly what need I hope.

Looks like we need to address the combination of LLVM_INSTALL_TOOLCHAIN_ONLY + LLVM_INSTALL_BINUTILS_SYMLINKS, so I opened upstream PR for that: https://reviews.llvm.org/D69635

If that method doesn't work then we can do what other distro do set LLVM_DISTRIBUTION_COMPONENTS then use install-distribution. See https://github.com/llvm-mirror/clang/blob/65acf43270ea2894dffa0d0b292b92402f80c8cb/cmake/caches/Fuchsia-stage2.cmake#L204

@sbc100
Copy link
Member Author

sbc100 commented Oct 30, 2019

@pchickey I think maybe you misunderstood what I'm suggesting. I'm not suggesting we stop shipping llvm/clang or stop building it, I'm suggesting that we maybe just ship the parts that we need (i.e. the toolchain parts, and maybe not the llvm development parts for working with llvm internals).

@pchickey
Copy link
Collaborator

Ah, yes, I did misunderstand. I agree we should not ship anything for working with llvm internals.

@DanielHoffmann
Copy link

I want to pinch in here and say it would be very useful to have a small lean SDK prebuilt binaries with everything packaged together. I was actually planning on writing a webpack loader for C++ files that would automatically install the SDK as a NPM postinstall script and nobody wants to download >300MB files on postinstall 😄

Many people from the JS world want to get into wasm and a webpack loader that takes care of installing the toolchain in a "NPM way" (ie, no external dependencies besides node) would dramatically lower the barrier to entry.

Also on another point having windows prebuilt binaries would be great too, I don't have access to a windows machine so I don't even know if the wasi-sdk builds to windows.

@pchickey
Copy link
Collaborator

Thanks for the input, Daniel. I am not an npm user but it sounds like thats a good use case for this tool.

Windows builds are currently blocked on a windows build bug in LLVM. I haven't looked into whether there is an upstream patch we can pull in there. We don't have any project maintainers who are currently working on windows, but we'd be happy for someone to take that on.

@pchickey
Copy link
Collaborator

Looks like there was just a 9.0.1 llvm release, we'll upgrade to that for the next sdk release. hopefully that fixes windows. Just leaving this note so I get back to it when I'm not on vacation :)

kildom pushed a commit to kildom/clang-wasi-port that referenced this issue Jul 14, 2021
* Add support for the Reactor model.

* Mark _activate and _start as wasm exports.

* Rename _activate to _initialize.

* Don't define `_fini`.

* Rename reactor-crt1.c to crt1-reactor.c.
@abrown
Copy link
Collaborator

abrown commented Aug 8, 2023

This issue is probably out of date by now; we can reopen if we want to revisit it but what is included in wasi-sdk has been stable-ish for the last year or so.

@abrown abrown closed this as completed Aug 8, 2023
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

4 participants