Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upHow to update to LLVM 3.5? #51
Comments
This comment has been minimized.
This comment has been minimized.
|
Hopefully this can happen eventually. But we should rebase on llvm svn. |
This comment has been minimized.
This comment has been minimized.
tomaka
commented
Sep 25, 2014
|
I'm eagerly waiting for this update in order to try making the Rust programming language compatible with Emscripten Someone (sorry, I don't remember the nickname) on the #emscripten IRC channel told me that he was working on it, I hope it's still the case. |
This comment has been minimized.
This comment has been minimized.
|
I'm also waiting for it. I can do it myself if having some description on how to do it. |
This comment has been minimized.
This comment has been minimized.
|
Emscripten-fastcomp is not a direct fork of upstream LLVM ( https://github.com/llvm-mirror/llvm ), but a fork of Google Native Client fork of LLVM. However Google uses depot tools and their repos seem to be all over the place, so this is not reflected in the github main page of this repository that it's a fork of another repo. I believe it's the PNaCl fork of LLVM (as opposed to the older NaCl project), and the upstream repository is probably this one: http://git.chromium.org/gitweb/?p=native_client/pnacl-llvm.git;a=tree;hb=HEAD . Although I am not completely sure, @kripken can verify. One could try to update with different strategies:
Also when merging, one must remember to merge both https://github.com/kripken/emscripten-fastcomp and https://github.com/kripken/emscripten-fastcomp-clang repositories. |
This comment has been minimized.
This comment has been minimized.
|
Thanks! I think i will find free time this week to do it and report you Anton. 2014-10-07 16:47 GMT+06:00 juj notifications@github.com:
|
This comment has been minimized.
This comment has been minimized.
|
While we have been based off of PNaCl, we should probably rebase off of upstream LLVM svn at this point, as I also just said on irc. Reasons are that PNaCl lags behind LLVM svn, and our usage of PNaCl passes has decreased to a small number. So taking our new JSBackend code, plus the few PNaCl passes we need, and rebasing that on LLVM svn, seems the simplest route. However, other options are fine too if they end up easier. |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 9, 2014
|
Done some quick investigation with some help from LLVM:
Clang:
I stopped looking at Clang because clearly the main effort is going to be moving LLVM. After squashing all emscripten commits I did some experiments:
I may have a look at how easy it is to lift the appropriate passes from pnacl. Without that knowledge the easiest way looks like rebasing onto pnacl llvm r34 and creeping forward to r35 when available. There's also the question of rebasing or merging. Merging seems like something that would work better in the long term given this 'rolling forward' will happen multiple times. |
This comment has been minimized.
This comment has been minimized.
|
Interesting, thanks for looking into this. I have heard pnacl is close to merging 3.5, at which point it might be easiest for us to base off of that, in collaboration with them. Yes, merging is probably better. |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 9, 2014
|
In which case I'll have a look at getting forward to 3.4, since (in theory) that should make the transition to 3.5 easier. |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 16, 2014
|
I'm down to 3 conflicts of .cpp files which require more knowledge than I have. I've created a repo to do the dirty work of getting to that point. If you clone https://github.com/aidanhs/rebase-emscripten and run script.sh it will
You'll be left with 3 *.rej files (patch hunks) which need to be reviewed, manually applied and deleted. The script is pretty simple and gives good guidance on what to for emscripten-fastcomp-clang. Let me know if you have any questions. |
This comment has been minimized.
This comment has been minimized.
|
Thanks! Taking a look now. |
This comment has been minimized.
This comment has been minimized.
|
Ok, I merged those 3 files manually. I committed and pushed the result to I didn't try to build yet though, as I noticed that lib/Target/JSBackend (our backend) is missing. Was that supposed to happen? |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 17, 2014
|
In addition to the stuff it puts in the staging area, you also should end up with a bunch of untracked files, including Looking at your diff, it doesn't look like any files have actually been added - |
This comment has been minimized.
This comment has been minimized.
|
Ok, thanks, I didn't do that important step ;) Done now, and pushed to that branch. Starting to try to build locally. |
This comment has been minimized.
This comment has been minimized.
|
Now that I'm back from vacation, I think I can land the PNaCl merge to 3.5 in the next few days, and then work with Alon to update Emscripten too. |
This comment has been minimized.
This comment has been minimized.
|
Ok, I pushed a few commits and LLVM builds. That leaves updating clang, which should be easier, maybe. After doing this work, I'm still a little unsure about whether rebasing or merging is going to be the best workflow, if this is to be a frequent thing. Needs more discussion. |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 17, 2014
|
I'll take a look at writing a script for clang. It should be much easier. The issue with rebasing and force pushing to the same branch is that you totally lose the ability to go back to a previous version, which is bad. Pushing to a new branch could be a solution but it makes it a bit mindbending to figure out the history of changes without a guide (same change on multiple branches on different bases!? Do you need to start thinking about backporting?). Unfortunately merges make history is a bit messy. I think I'd prefer it over the alternatives though... |
This comment has been minimized.
This comment has been minimized.
|
I would definitely favor merging - as long as merges flow one direction 2014-11-18 0:59 GMT+02:00 aidanhs notifications@github.com:
|
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 18, 2014
|
Script done. Same repo (https://github.com/aidanhs/rebase-emscripten), now imaginatively called 'script2.sh'. There weren't any tricky merge conflicts, so you shouldn't see any .rej files listed at the end, i.e. you can just add, commit and try it out. |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 18, 2014
|
Just had to tweak the script, re-pull if you've already grabbed it. |
This comment has been minimized.
This comment has been minimized.
|
How do you find the 'base' commit in each case? That is, the point where "emscripten" stuff starts, that you roll back to? |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 18, 2014
I then use gitk to manually check the branch graph to make sure it looks sane. |
This comment has been minimized.
This comment has been minimized.
|
I see, thanks. Ok, after thinking some more, and chatting with @jfbastien about how they merge llvm into pnacl, I think the merge approach is better. I'll try to do that today. Hopefully it will be as simple as just doing the merge and then applying the diff that you made here, plus the other changes I did on top of that. I also think we should do this on 3.4, as we are doing now, then let that stabilize for a while before tackling 3.5. |
This comment has been minimized.
This comment has been minimized.
|
Ok, after the work done here so far, this was fairly easy. I got it to build ok and pushed |
This comment has been minimized.
This comment has been minimized.
|
I am running into a git issue maybe someone here can fix for me. I have a merge commit, then I wrote some commits on top of that. I want to squash the commits into the merge commit (so that bisection will work). However, e.g. |
This comment has been minimized.
This comment has been minimized.
|
Hmm, not sure about interactive rebase, but this probably will work:
The soft reset will not touch the working tree, but changes the commit that the currently checked out branch points to, so it will look like as if you just authored the changes to the working tree. Then an amend commit allows you to fuse those changes into the merge commit, as if you had originally written them before committing your merge. |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 19, 2014
|
As always with git, there are many ways to do it. I don't specifically know how to make rebase behave, but as an example of what you could do (I tend to prefer the manual routes of diffing and applying):
You could shorten this by doing |
This comment has been minimized.
This comment has been minimized.
aidanhs
commented
Nov 19, 2014
|
The reset --soft idea is pretty cool actually, probably a better approach. |
This comment has been minimized.
This comment has been minimized.
|
Thanks, now I think I finally understand what --soft is good for ;) Will try this later today, still some test failures happening. |
This comment has been minimized.
This comment has been minimized.
|
Main test suite now passes. I disabled some things in non-fastcomp to get there. Running fuzzing and the rest of the test suite now. Hopefully I can merge this to incoming tomorrow. |
kripken
referenced this issue
Jan 7, 2015
Merged
Update embind to work with the Closure Compiler #3084
This comment has been minimized.
This comment has been minimized.
|
libc++ patch looks tiny and reasonable, and fixes the embind problem. Pushed to incoming. Hopefully they'll apply this upstream soon, but it's a pretty small divergence for now. |
This comment has been minimized.
This comment has been minimized.
tomaka
commented
Jan 7, 2015
|
Sorry for the noobish question, but I tried running
("référence indéfinie" means "undefined reference") I was successfully building emscripten before (including after the 3.4 merge), so I suppose that this problem comes from the 3.5 merge? |
This comment has been minimized.
This comment has been minimized.
|
Yes, likely a merge issue. Can you run configure or cmake again, then try to build? Maybe a major upgrade of LLVM needs that. Also, which of those two are you using, and with what flags? |
This comment has been minimized.
This comment has been minimized.
|
@tomaka - interesting, I think I see your error on the OS X bot. Are you on OS X too? Perhaps that's why I don't see it on linux. |
This comment has been minimized.
This comment has been minimized.
ibdknox
commented
Jan 8, 2015
|
I'm getting build errors on OSX as well. Here's what I'm getting on two different machines that both had previous versions working:
|
This comment has been minimized.
This comment has been minimized.
tomaka
commented
Jan 8, 2015
No, I'm on Ubuntu. I cleaned the build directory and executed |
This comment has been minimized.
This comment has been minimized.
|
Ok, maybe the difference is CMake vs configure - I use configure locally. Will test CMake tomorrow. @ibdknox , is that error with CMake or configure? |
This comment has been minimized.
This comment has been minimized.
ibdknox
commented
Jan 8, 2015
|
that's with CMake |
This comment has been minimized.
This comment has been minimized.
floooh
commented
Jan 8, 2015
|
I'm getting the exact same errors as @ibdknox on OSX 10.10 when linking 'opt' with a fresh pull from incoming in the emsdk, I'll try on my Linux VM next. I'll see if I can investigate a bit more in a few hours. Linking CXX executable ../../bin/opt |
This comment has been minimized.
This comment has been minimized.
floooh
commented
Jan 8, 2015
|
I'm also getting those errors on my Linux Mint VM (compiled with "Ubuntu clang version 3.5-lubuntu1". Just a shot in the dark, but the git log says that the merge had conflicts (among others in tools/CMakeLists.txt and tools/opt/opt.cpp), maybe something went wrong when resolving the conflicts? It looks like the the opt-tool is missing one or more link libs? |
This comment has been minimized.
This comment has been minimized.
floooh
commented
Jan 8, 2015
|
PS: also CMake compile here, that's what the emsdk uses automatically. |
This comment has been minimized.
This comment has been minimized.
|
Thanks everyone, I did fresh builds locally with both configure and cmake, and I can confirm the cmake error now. |
This comment has been minimized.
This comment has been minimized.
|
Ok, I pushed a commit that fixes this locally. Oddly it doesn't look like a merge issue - that broken CMakeLists.txt file wasn't in the conflicts. @jfbastien , do you test CMake builds in PNaCl, or just configure? |
This comment has been minimized.
This comment has been minimized.
tomaka
commented
Jan 8, 2015
|
The fix works for me too. |
This comment has been minimized.
This comment has been minimized.
ibdknox
commented
Jan 8, 2015
|
@kripken fixed here as well. |
This comment has been minimized.
This comment has been minimized.
|
Great, thanks for the confirmation @tomaka and @ibdknox . Hmm, meanwhile I see on the bots that the windows one fails on
@juj, when you get back from vacation, hopefully that's not hard to upgrade? |
This comment has been minimized.
This comment has been minimized.
|
Bots (OS X and linux) passed linking of |
This comment has been minimized.
This comment has been minimized.
|
@kripken we test with configure on the bots because some of the tests can't be run under cmake, but some of us build with cmake locally so I expect it to work (it does locally). Anything we should patch? |
This comment has been minimized.
This comment has been minimized.
floooh
commented
Jan 8, 2015
|
Compiling here on OSX too, but I got a nasty crash when trying to compile my engine for the first time, it was one of the source files, not the librarian or linker (unfortunately not reproducible on the 2nd and 3rd try). I had jekyll and a few VMs running in the background so may be memory was low, still it's the first time clang ever crashed on me. Here's the crash log with call stack, I'll have an eye on it whether it happens again, guess that's something for the clang team to look at? |
This comment has been minimized.
This comment has been minimized.
|
@floooh do you have a repro? This does seem like a clang bug. |
This comment has been minimized.
This comment has been minimized.
floooh
commented
Jan 8, 2015
|
@jfbastien no repo so far, I compiled the whole code 4 times so far and the crash didn't happen again. if anyone wants to give it a try (on OSX or Linux, with python 2.7 in the path):
I'll see if I can setup a stress test and recompile in an infinite loop over night or so... (Update: the ./oryol setup was wrong... and the ./oryol build too) |
This comment has been minimized.
This comment has been minimized.
|
@jfbastien I had to do this: bbe4691 looks like on master of pnacl-llvm you would only need to add 2 of those,
|
This comment has been minimized.
This comment has been minimized.
floooh
commented
Jan 8, 2015
|
PS: the PNaCl pepper_canary version also compiles so far (first "./oryol setup nacl", then "./oryol build pnacl-make-release"), @jfbastien, do you know when the nacl SDK switched to clang 3.5? I've been compiling both the emscripten (with clang 3.4) and pnacl version very frequently in the past weeks, and didn't have problems on this code base. |
This comment has been minimized.
This comment has been minimized.
|
@kripken looks like I fixed that one here: https://codereview.chromium.org/749363002/diff/1/tools/opt/CMakeLists.txt @floooh 3.5 will be in the SDK as of version 41. It's currently in pepper_canary, and Chrome should be branching tomorrow, hitting stable in early March. |
This comment has been minimized.
This comment has been minimized.
|
OS X bot is now green, and Ubuntu looks fine so far. I think we are good, except for the windows bot which fails as mentioned before. |
This comment has been minimized.
This comment has been minimized.
|
This can be closed now? |
4ntoine commentedSep 24, 2014
No description provided.