-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Add necessary patches for LLVM 3.7(.1) #14430
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
Conversation
|
3.7.1 was already tagged but isn't up at http://llvm.org/releases/3.7.1 yet, so I think we should skip the 3.7.0 checksums. I have a test branch where now we can actually just build llvm from source and cache the result on Linux Docker Travis. Mac will need a homebrew bottle. edit: and AppVeyor will need me to build win32 and win64 binaries and put them up on bintray. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this in the diff? we don't even use landingpads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Miscompiles Cxx otherwise.
|
Need to ping llvm-dev about putting up source tarballs. I don't get what in their release process takes so long after tagging (http://lists.llvm.org/pipermail/llvm-dev/2015-December/093179.html was 10 days ago). |
|
Not sure what's going on with 3.7.1. Let's just go with 3.7.0 for now and switch later. |
|
It's more of a time investment for me to rebuild binaries across 2-3 different platforms than it is to get the llvm release manager to finish the job here. I'm not subscribed to llvm-dev, can we ask what the holdup is here? |
|
This thread was started yesterday: http://lists.llvm.org/pipermail/llvm-dev/2015-December/093572.html |
|
It was suggested that we switch LLVM to git-external to be able to pull in the release branch even though the tarballs aren't up yet. What would we do about the patches? Those are based on the LLVM version which we wouldn't know any more. |
|
Can't we just create a source tarball of 3.7.1 on our own and host it on s3? If llvm.org releases a tarball, we can switch to it. |
|
Yeah, I'm in favor of that solution as well. Let's just do the switch. It's up to @tkelman though. I'm happy to do whatever is required to finally get this done. |
|
With a git external we'd be using sha instead of version number, so the patch application should be tweaked to also check for that. Using a different url for one specific version is pretty messy, but maybe the fastest way to get something temporary in place. |
|
Why is it so messy? It just changes a URL for download that can be reverted when the official tarball is made available. Seems like the smallest change to get this going. |
|
Ok, S3 mirror is up. Should be good to go. @tkelman shall we merge this, so you can prepare the buildbots and then we'll do the actual switch in a separate PR? |
|
I think you need to fix the permissions so that bucket is readable. What are the chances your tarballs will end up identical to the real ones once they go up? |
|
I thought the bucket was readable. I tested with this branch, but that was from within AWS, so maybe that was the difference. |
|
Ok, try again. The files were public, but I hadn't made the bucket listable. |
|
Oh, and I think the content is the same (this is the final tags), but I don't know how the release tarballs are built so I'm pretty sure they won't be bit-identical. |
|
we should make the directory name inside the tarball consistent with upstream releases, so llvm-3.7.1.src |
|
The release win32 build, but not the debug build, segfaults at the start of bootstrap on this branch with llvm 3.7.1. edit: and the debug build segfaults during second stage inference.ji at range.jl, line 718 |
|
win64 is fine? |
|
will let you know when it finishes |
|
Also, what gcc version? Anything > 4.9 miscompiles llvm. |
|
4.9.2. That's a bit of a problem, given clang as a mingw-w64 cross compiler isn't especially easy to set up or use. Can we patch llvm or add a gcc flag to work around it? Fedora and Arch are also on gcc 5 by now afaik. |
|
Yes, you can use |
|
What would be the downside of adding that flag anywhere we're using gcc? Is it needed just for llvm, or libjulia, or (somehow) all deps? |
|
Probably fine to add it unconditionally when targeting mingw. Do we have a way to detect compiler version? |
|
We need > 4.7 due to c++11 in new llvm anyway. Looks like there's a proposed patch for the gcc issue at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66655, just need to build mingw to test it. win64 actually looks okay, tests are passing and performance looks normal. getting |
|
Ok, I'll take a look at the win32 failure. |
|
sorry wrong button. given a9b3015#diff-9, may want to manually define USE_ORCJIT if we know our homebrew llvm is patched |
|
I'm suspecting the remaining win32 failure to be another miscompilation that's not fixed by the indicated workaround. I'm actually quite speechless as to how bad gcc has gotten in version 5. I think we should move ahead anyway, and just tell people not to use |
|
4.9.2 from cygwin actually looks like it's working now. Which toolchain were you using for win32, opensuse or the one from msys2 pacman? What kind of failure are you seeing? I used the opensuse build service to apply the suggested patch from the gcc bug thread and it fixed the test case, so hopefully that may get applied before gcc 6 and/or 5.4. Could also direct the |
|
msys2 pacman gcc 5.3.0. I'm seeing LLVM being miscompiled. Easily checked by running the llvm tests to see if they're failing. |
|
"our build system" does not use msys2 pacman's gcc. On win32 that's also dwarf exception handling, so I'm surprised it built at all. I do still see which looks similar to #11083. LLVM tests don't run particularly easily here, |
|
I realized I was wrong about my gcc. It's the one we get using |
|
@tkelman I can't reproduce the subarray error. |
|
try running tests in a single process with |
|
@staticfloat @tkelman This seems to require diving into a fair amount of homebrew, which I am happy to do, but won't be able to for all of the coming week. |
|
I can believe that |
|
It isn't broken now and it is broken under llvm 3.7, so it's definitely a regression. |
|
Ok, I can try to do to improve the situation, but I don't think we should hold up the transition on other platforms because of it. |
|
squash out commits that fail to build, fix the folder name in the tarballs (and adjust the checksums accordingly) then I think we can merge this and move to the next step. |
And make ORC JIT the default for those LLVM versions. This does not activate the new LLVM versions by default yet, to give time to update everything on CI, etc., but doing the actual activation is a one line change after this.
Make jl_setup_module idempotent and remove a bunch of ad-hoc code that was supposed fix up the module after creating the execution engine. In the process, fix a bug where not all modules would have the layout set, causing mis-optimizations.
|
Done |
Add necessary patches for LLVM 3.7(.1)
|
aaand they finally put up the real release today at http://llvm.org/releases/3.7.1. Ping me when homebrew stuff is getting ready and I'll do a PR to switch the CI on all 5 main platforms. edit: https://github.com/Homebrew/homebrew-versions/blob/master/llvm37.rb is probably the best place to start, then add our patches |
And make ORC JIT the default for those LLVM versions. This does not activate the new LLVM versions by default yet, to give time to update everything on CI, etc., but doing the actual activation is a one line change after this. (cherry picked from commit a38fa5f) ref JuliaLang#14430
And make ORC JIT the default for those LLVM versions. This does
not activate the new LLVM versions by default yet, to give time
to update everything on CI, etc., but doing the actual activation
is a one line change after this.
@staticfloat @tkelman Once this is done, how do we go about
preparing our CI infrastructure for the switch?
Close #14466