-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Upstream the llvm patches #4259
Comments
Is there any progress or timeline when you want to push your changes to upstream? |
Referencing #5368. |
@fabiand we actually are carrying no key llvm patches at the moment besides those dealing with ARM. I don't know the ETA for upstreaming those. |
@brson That's good to know. Is there currently a build switch to build rust without pulling and building llvm? (So using existing llvm binaries?) |
@fabiand |
@brson, thanks - It doesn't seem to work. The linker can't find the installed version. But I did not yet nail it down further. THere was one patch needed for llvm3.2 (wllvm32 branch) |
Not for 0.6. |
I just wanted to jump in and say that LLVM is in the midst of planning their next release. If you have any patches for LLVM, please let us know! :-) Either by creating a PR or sending them to llvm-commits. Thanks. :-) |
Nominated for milestone due to time-sensitivity. |
visiting for triage email 2013-07-22. @brson @graydon : Any way we can up the priority on this task? Do we think we missed the window of time described in isanbard's comment? From my reading of the releases at the LLVM download page and the LLVM release process docs, it seems like they release every 6 months, and we might expect the next release to take place to occur in December. (Maybe isanbard was referring to the 3.3 release, which came out on June 17th.) So this may not be super-duper high priority, but that might be all the more reason to actually start working this through our own pipeline... |
Unfortunately, you did miss the window for the 3.3 release. The next major release is around the end of the year. Though there may be a "dot" release before that. (I believe one of our contributors is in the middle of doing this.) |
Are we also certain that we won't have more patches for LLVM, what with the new scheduler, actual GC, TBAA, etc.? |
We don't need any patches for a TBAA pass, it can be loaded as a dynamic library by LLVM. |
There will probably one more patch for #1226 (unless upstream has already done it). Additionally, I think that the patches we do have are probably not up to the quality standards llvm expects (missing tests, etc.). |
@pnkfelix I haven't been very motivated to pursue this, sadly. It will be significant work, we still have at least some known new patches to write (mentioned above), the patches we do have need to be improved, and from what I understand some of our arm patches are somewhat special-cased and likely need further design work. |
Also, last time I tried to upgrade LLVM I hit errors on windows, and as that so often goes, I just gave up rather than try to debug there. |
Certainly we, the Rust project, have to maintain our own fork, it's still good for the upstream, other projects and package maintainers to merge patches into the upstream as soon as it gets mature, isn't it? |
@omasanori we don't need our own fork, the only reason we do is to work around issues in llvm |
@cmr Certainly it's not a complete fork. |
The intention is for Rust to be using a vanilla LLVM; we shouldn't have our own fork. The current patches can be reworked until they're accepted upstream or just dropped by removing the dependency on them. |
As of right now, here's a list of patches we have to upstream. Of those, two can be ignored. One is already upstream (Use a mutex instead of an RWMutex), and the other is a fix to get around a bug in 3.3 that was already fixed upstream (Skip i8 case in fastisel) |
A fellow Debian and LLVM upstream developer agreed to take a look at our patch queue and integrating the non-debatable ones upstream. I'll keep you posted on the status. |
☀️ wonderful! |
🎉 (icons!) I'd love to get an official package in place for Fedora 21 |
@lucab, awesome! If you want to connect me to them or vice versa, I've been dealing with the LLVM upgrades recently so I could try to help out where needed. |
@lucab Any updates? Also, I'm curious what the cutoff is for getting patches into the next stable LLVM release. |
@sylvestre said he would have taken care of this, not sure about the status right now... |
This is the documentation on how to write them: http://llvm.org/docs/DeveloperPolicy.html#test-cases It's not very extensive, but there are many examples in the code base of tests (see the test/ directories). Just use good testing practices to figure out what to test. |
Nominating. Is it a goal to be able to use system LLVM for 1.0? |
@brson: it would need to happen to have a distribution like Fedora or Debian to package it, so I think it should be |
Indeed. I haven't followed that closely but is the bootstrap "issue" been fixed ? |
@sylvestre What is the "bootstrap issue"? |
As explained here https://github.com/mozilla/rust |
The snapshot can be downloaded in advance and this definitely works as Rust won't keep downloading it on each build if |
@sylvestre I'm collecting some preliminary ideas on that at https://wiki.debian.org/Teams/RustPackaging/Bootstrap but that's not the point of this bug report. The LLVM upstreaming issue is a separate one which also the Fedora guys would like to have addressed. I believe this is not an hard requirements for both distro, but will make package review harder (nobody wants yet another full LLVM copy in the archive). |
OK, thanks mate :) About the bug, if there is no tests, it is going to be impossible to see the patches applied upstream. |
The If stop using the segmented stack support, then most of this problem will go away. |
We discussed what our requirements with respect to this issue in the weekly meeting. Our current stance is this: We want to be compatible with a reference host-provided LLVM for Rust 1.0, and we believe that this is an achievable goal (at least on x86 hosts). So, we will attempt to support and/or shepherd-in changes that move us towards the goal of being able to work with a host-provided LLVM. (e.g. @alexcrichton has some ideas on how to deal with Bu: working atop a host-provided LLVM is not something that should block the Rust 1.0 release. |
(removing the I-nominated tag since we have now had the delayed discussion of the matter.) |
@alexcrichton what ideas are those? |
Any |
@alexcrichton: They expressed that they wanted |
The win64 segmented stack patch was just upstreamed, I've updated the issue description to reflect remaining work. |
The segmented stacks for ARM patches have been upstreamed. |
The equivalent of a no-split-stack patch has been upstreamed. On the next LLVM update we will have to change our codegen to attach the |
The last remaining patch is just an optimization, and I don't think it's critical that we upstream it. We can keep it around locally for awhile, however. We've got a bit of work on our hands to get compatible with upstream LLVM 3.5, but it'll be hairy regardless because we'll have to start dealing with c++11 compilers and such. With all the patches upstreamed, I'm going to close this issue. Hurray! |
\o/
|
👍 |
👍 |
@alexcrichton This (and the libuv too) are great news. Thanks for all your efforts! |
* Fix newlines in JSON output This changes the JSON output to be more consistent about where newlines are included. Previously it only included them between lines in a multiline diff. That meant single line changes were treated a bit weirdly. This changes it to append a newline to every line. When feeding the results into `arc lint` this behaves correctly. I have only done limited testing though, in particular there's a possibility it might not work with files with `\r\n` endings (though that would have been the case before too). Fixes rust-lang#4259 * Update tests # Conflicts: # tests/writemode/target/output.json
We have a handful of llvm patches that need to be pushed upstream. llvm just had a release, so have a nice window to get our patches reviewed and integrated for the next release.
Remaining patches to upstream
ARM segmented stacksno-split-stackwin64 segmented stacksgetPassInfo()
(optional)The text was updated successfully, but these errors were encountered: