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

WIP: switch emscripten backend to llvm #44846

Closed
wants to merge 1 commit into from

Conversation

igas
Copy link
Contributor

@igas igas commented Oct 3, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

@igas
Copy link
Contributor Author

igas commented Oct 3, 2019

@kripken I kicked off, but not sure I understand the whole build process. resource blocks is just download them. later we put them to the paths under workdir (emscripten-core/emscripten)

(buildpath/"fastcomp").install resource("fastcomp")
(buildpath/"fastcomp/tools/clang").install resource("fastcomp-clang")
. From here I'm not really sure how it's built as I'm not familiar with either python to understand built script or cmake :(

If there are approximate list of shell commands I can translate it to brew.

@chrmoritz
Copy link
Contributor

chrmoritz commented Oct 3, 2019

Something you are definitely missing here is the recursedeps = [ 'v8' ] at the end of the DEPS file, which means that you would need to also include all the deps from v8's DEPS file for that specific commit (basically at least the stuff we have in the v8 formula too relative to the v8 dir).

But I'm not sure if all these deps are actually required at runtime. You might only need llvm and binaryen.

Yeah, waterfall is their CI infrastructure tool, fastcomp is only needed for the old backend and v8 is there for running a integration test I guess and wabt could probably be a separate formula on it's own (wait it already is, so that's also not needed here).

So whats need to be done here is to build the specific commits of llvm and binaryen from source and putting everything in the right places or at least edit .emscripten to point to the right places.

@kripken kripken mentioned this pull request Oct 3, 2019
@kripken
Copy link

kripken commented Oct 3, 2019

Correct @chrmoritz, fastcomp is only needed for the old backend. Yes, in general you just need builds of llvm and binaryen. And also there is a dependency on node.js and python. (Optionally Java for closure compiler too.)

I wonder, btw, if it's possible to wrap around the emsdk for this? Getting the latest emsdk and running

emsdk install $VERSION-upstream
emsdk activate $VERSION-upstream

will get everything from our build infrastructure, and store it in under that directory. Then you can just use llvm, binaryen, emcc, etc. from there. That seems simple, and also those builds are what is constantly tested by us, and we have full mac, linux etc. support. Is brew wrapping around the emsdk like that an option?

@chrmoritz
Copy link
Contributor

chrmoritz commented Oct 5, 2019

I wonder, btw, if it's possible to wrap around the emsdk for this? Getting the latest emsdk and running

I think this wouldn't be possible in homebrew, because it installs precompiled binaries, which isn't acceptable for a core formula.

@igas I've also tried to make it working and here is my progress so far: chrmoritz@7cff243. Not sure what llvm args are actually needed because we only need the host;WebAssembly targets and these arguments are more or less copied from the normal llvm formula so far.

@igas
Copy link
Contributor Author

igas commented Oct 8, 2019

@chrmoritz I'm happy for you to take over this as it's not my area of expertise at all 😅 closest thing to the compiled languages I worked with was erlang ^_^

@chrmoritz
Copy link
Contributor

I'm not so familiar with all the llvm stuff either. Especially I don't know the reasoning behind all these cmake arguments in the llvm formula. That's why the args used in my attempt were more or less a mix between the ones used in upstream's build.py and striped down version of the one we have in our formula (we need only 2 targets: host;WebAssembly here).

But except figuring out the best args passed to llvm's cmake and actually carefully testing this, it should already somehow work that way.

@SMillerDev
Copy link
Member

@BrewTestBot test this please!

@openingnow
Copy link

Is this PR still active? I want to make a commit solving #47869 but it looks like handling formula will make conflict.

@openingnow
Copy link

I think this wouldn't be possible in homebrew, because it installs precompiled binaries, which isn't acceptable for a core formula.

What about providing emsdk formula instead emscripten and then make user to install emscripten, like haskell-stack do?

@chrmoritz
Copy link
Contributor

What about providing emsdk formula instead emscripten and then make user to install emscripten, like haskell-stack do?

Why not building a minimal llvm and binaryen from source in the formula for their specific revisions corresponding to the emscripten release. I mean the current version of the formula is also building fastcomp from source, so we could do the same thing for the new dependencies.

If you want you could take over where I stopped in chrmoritz@7cff243 (but you will still have to improve the args passed to llvm a bit I think).

@stale
Copy link

stale bot commented Jan 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale No recent activity label Jan 8, 2020
@stale stale bot closed this Jan 15, 2020
@lock lock bot added the outdated PR was locked due to age label Feb 14, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants