-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Bump minimum LLVM version to LLVM 6 #55842
Comments
cc @djc |
I think there is a bit of a misapprehension around how the rust LLVM fork works. The fork basically only contains two types of patches:
There are only two commits that do not fall into this category:
I'm writing this to clarify that generally the flow of patches is always from LLVM upstream to the rust fork, not the other way around. I think that the issue that you have in mind is really a different one, namely that we don't really keep track of which upstream patches are needed for what over time, and also don't request necessary upstream patches to be merged into previous LLVM point releases. (Definitely +1 on dropping old LLVM support though, as long as distros are fine with it.) |
LLVM 5.0 is barely a year old, and you are suggesting to abandon it already. One reasonable LLVM update policy is to migrate from X.0.1 to X+1.0.1 for reliability, rather than from X.0.0 to X+1.0.0. |
To put things in perspective with respect to the release train: LLVM 5.0 was released in August 2017. If we decide to bump the minimum version, until the PR lands on nightly we are talking end of November 2018, 6 weeks later it lands in beta (mid January 2019) and 6 weeks afterwards in stable ~March 2019. By March 2019 LLVM 5.0 will be ~1.5 years old., and Rust master will be using LLVM 9@trunk.
cc @kripken ( |
We recently upgraded to LLVM 8 (based on 3480ac2) thanks to @TimNN, so I don't believe we are a concern here.
This has been the AVR fork's intention as well — we try to only have LLVM patches that are already in LLVM master, or at least open for review to be merged. |
As you mentioned, Fedora 28 is still on LLVM 6, and it's non-trivial to upgrade the main LLVM package for the distro. And now that 29 is out, we'll be trying even harder not to rock the boat on 28, as many people use release-1 as a more conservative (stable) Fedora choice. If hard-pressed, I could add an llvm7 compat package, leaving the main one alone -- but that's not usually done for newer versions, and would present new difficulties for system upgrades. I could also build with the bundled LLVM if really necessary.
Latest 2 LLVM versions would be better. For Fedora, this lines up nicely since we also release every 6 months and support the latest 2. (Although we actually support 3 for a window of about a month -- e.g. Fedora 27 isn't quite EOL yet.) It would be even better if you wait for the stable patch release, giving more time for new issues to get worked out. We haven't even seen 7.0.1 yet, but that's in RC now. Ideally, this time until the patch release is a good time for new Rust backports on LLVM X.0 to also be suggested for the official LLVM X.0.1 release. I do try to work with @tstellar already to make that happen.
I don't think Rust should make a habit of jumping onto the latest trunk. I understand this was done currently for some WASM fixes, but this should be an exceptional case. Even right now, what we're calling "LLVM 8" is just a snapshot that's actually closer to 7 -- I count ~1200 commits past It's well and good to make a best effort to support building with trunk, so we're ready for new releases sooner, but in general I think we should wait for the actual release before we move. |
@cuviper Could you mention why is it worth it to avoid the bundled LLVM ? Other distro packagers mentioned in the internal threads that they have decided to always use the bundled LLVM anyways.
IIRC LLVM recommends projects that depend on it but are hosted outside their main repo to follow trunk as closely as possible because its C++ API does not guarantee any kind of backwards compatibility and this allows new issues to be catched with enough time to fix them before each release.
If we decide to go this route, it might be worth it to add a build bot that attempts to build rustc with LLVM trunk on some tier 1 targets (e.g. |
It's a general policy: https://fedoraproject.org/wiki/Bundled_Libraries And just as a practical matter, it really helps build times to not have to build LLVM too. We don't cache any artifacts between builds like Rust's CI does, and
I read it that they've resigned to using the bundled LLVM, not that this would be preferred.
It's a mixed bag, because this also means that fewer people would be testing the release. @tstellar had a talk at LLVM-Dev 2017 advocating more people use stable versions. Even still, we're not actually following trunk closely, as I pointed out how far behind we are right now. We can still give that upstream benefit of testing pre-releases of LLVM before it gets out the door, but I don't think we should be releasing Rust based on unreleased LLVM. |
Thank you, all of that makes sense!
To implement this in a useful way we would have to stop making rustc require pre-release LLVM versions to work properly. |
So, as a Gentoo packager, here's my take:
I very much support this line of thinking. |
That is, if currently Rust is using LLVM8@trunk, would LLVM7 be enough? Or would you rather have LLVM6 ?
Would it help if, e.g., the release notes would contain the LLVM version + commit of the bundled LLVM used for stable Rust releases? (maybe with a link to the branch uses by rust's bundled LLVM fork?)
@cuviper had the same request and this is a pretty hard constraint. To expand on this:
Any solution would have to make things work for distros, without making anything worse for those using the bundled LLVM. A "compromise" might be to use LLVM@trunk on nightly, and to also test beta thoroughly with the minimum supported LLVM version (on all targets). This gives us 6 weeks to error properly on everything that requires the bundled LLVM or to try to provide a backwards compatible workarounds. But 6 weeks is not a long time and the older the minimum released LLVM version is, the higher the chances that Rust users on stable that are not using the bundled LLVM will just see an "error: your system's llvm version is too old"-type of error message. |
This reminds me... |
FYI, #55835 is updating to a newer trunk snapshot. |
FYI, to put that in context, there are people collaborating to specify 128-bit SIMD for WASM, implement that in LLVM, and exposing that from Rust. This is iterative work that requires keeping the spec, llvm, and Rust, in sync. And this is not the only feature that requires this kind of synchronization. So rust-lang/master following LLVM trunk is pretty much a hard requirement for progress in some fronts right now. As long as these features do not percolate into stable, this does not imply that Rust stable releases have to require LLVM trunk as well. I believe we can find a solution that works for all parties involved. |
I was talking about having LLVM 6. But to some extent, that might be a longer-term desire. It's much more important to me that we get to use (something close to) an actual LLVM release for stable Rust builds.
For stable Rust, it would still be disappointing if we had to pull a "random" LLVM commit from upstream. For nightly Rust (which we don't package yet), that might be acceptable. For stable Rust, it would be better if there was something like "this released version + these upstream commits targeting these particular bugs", as I outlined in my previous comment. That would allow us to weight cost/benefit for each of the extra patches separately (for example, some of them might affect Windows only, which for Gentoo we wouldn't care about so much).
Testing with a released version of LLVM on the beta branch sounds like a potentially big improvement over what we have now, from my perspective. IMO that's a worthy goal even if it is the very latest LLVM released ever at the time. |
I'm not super involved in these matters but my impression from the issues I see filed against Rust and later categorized as LLVM bugs is that a lot of issues are not caught by our existing tests when we upgrade LLVM but rather found in the wild by end users. So I'm scared that shipping nightlies with bleeding edge LLVM and stable with an older LLVM would just lead to taking longer to notice issues with said older LLVM and to stable releases being less reliable. In theory there's the beta period, but the bugs already sneaking into stable today indicate that beta is already not tested enough, so relying on it to catch even more (and often very subtle) issues is a scary prospect to me. |
I wasn't suggesting this. I was suggesting that beta could also be tested with the minimum LLVM version that we support. Some distros are already shipping stable Rust releases that do not use the bundled LLVM, they are using this to compile code that they are also shipping to their users, and I suspect that no distro is testing this properly. Reducing the minimum supported LLVM version that's 1-2 releases old is an improvement. Running some more tests on beta using the minimum supported LLVM version would be an improvement as well. Are these changes enough to make Rust toolchains that use the system LLVM reliable ? No, but using the system LLVM is an use case that we currently do not block, and I don't think this is the place to start a discussion about this. It appears that bumpting the minimum supported LLVM version to 6.0.1. has some consensus. I'll try to get to it this week and will ping the packagers I know there so that you can give the PR a try (there should really be a @rust-lang/packagers or similar group to make this easier, I voiced this in the community discord channel and the community team appeared to agree). |
I already addressed that beta would also be tested, and why I don't think that's good enough. It's true that it would still be an improvement for distros that already ship a rustc using an old LLVM release, but
|
Why ? |
Ok then I must have misunderstood. If we don't ship rustcs compiled against old LLVM releases to users, then how do you propose we test them more exhaustively than we already do? Where do we get those extra test cases? We run in-tree tests with an old LLVM on CI and crater persumably already ran back when we first upgraded our bundled LLVM to approximately the release that is now our lower bound. |
We currently only have one build bot ( There are also a lot of dependencies that rust-lang/rust assumes to work correctly (libc, stdsimd, jemalloc-sys, rayon, miri, chalk, clippy, rustfmt, etc.). None of these is tested against the minimum LLVM AFAIK (not by us, not by the distros themselves). So adding more build bots to beta that test a couple of more platforms with the system's llvm, and that also run tests for these components using the system LLVM, would be a way to make rust toolchains using the system llvm more reliable. In any case, there seems to be consensus that bumping the minimum LLVM to version 6.0.1 is "ok", so I'll try to do that in the next days. I don't think that we did a crater run last time that we updated this, and I don't even know if we can do crater runs using the system LLVM, but I will ask about this. |
For what it's worth I would like to jump in and say that I've run into a number of problems relating to rust's llvm bitcode not being compatible with the version of llvm it purports to be shipping, presumably because of these upstream patches. This creates serious difficulties for those of us who are trying to integrate rust code into projects written in multiple llvm languages. It would be a huge benefit to the project I'm working on if it were possible for there to be a release of rust that doesn't include patches that break the format of llvm bitcode. |
@dwightguth None of the LLVM patches used by Rust should be introducing bitcode incompatibilities. Quite likely something else is at fault there. Could you maybe open a separate issue about this with the incompatibilities you're seeing and the LLVM versions involved? |
This may also be because rust-llvm is not always a true LLVM release. What we're calling LLVM 8 right now is really just a snapshot from trunk. |
See #55842 (comment) |
@dwightguth If you're interoperating with clang in particular, #56371 will probably help your use case, once implemented. |
For reference, does Clang have a policy of supporting older versions of LLVM? Or does each Clang release pin to a specific version of LLVM, and if you have an old LLVM version you just use an older Clang? I suppose it's quite easy to believe that infrequent compiler updates are more tolerable in C++ than in Rust. |
Clang is part of LLVM, it’s in the same repository, they are part of the
same binary release.
Comparing projects that are in LLVM with projects outside does not make
much sense imo. Two completely different worlds.
…On Fri 30. Nov 2018 at 20:29, Ben Striegel ***@***.***> wrote:
For reference, does Clang have a policy of supporting older versions of
LLVM? Or does each Clang release pin to a specific version of LLVM, and if
you have an old LLVM version you just use an older Clang? I suppose it's
quite easy to believe that infrequent compiler updates are more tolerable
in C++ than in Rust.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#55842 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3NproHWocLIdsa6FyLlLl3_WGe_mfEks5u0YcagaJpZM4YX5JT>
.
|
From the LLVM Developer Policy on IR backwards compatibility
To summarise, a binary bitcode file The textual representation of that same LLVM IR, a I had a quick look at the rust-lang/llvm top of tree. None of the Rust-specific patches touch the |
This is true to the point that even |
It sounds like best way to proceed is to vastly expand CI coverage, and use Cargo features to disable things (usually language features) based on LLVM version. This sucks, but makes clear the actually costs of supporting multiple versions rather than keeping it with allowed test failures and missing CI combinations. Also, as a contributor to NixOS/Nixpkgs, most distributions in supporting supporting only 1 version of things (and a stable-1 version at that) seems awfully conservative. I get that Rust is constrained by the popularity-contest-winning/incumbent distros, but just wanted to pipe up that distributions don't always be the enemy of agility. Now that everything isn't barely tested C, we don't need to be so skiddish, or can at least offer users options to choose from given their own risk tolerance. FWIW My skin in the game is hoping to see #52694 fixed, so we can proceed on allocator polymorphism. Gating an unstable library feature on the LLVM version (perhaps indirectly via a language feature), is a perfect easy and sound workaround that allows exploration without picking a fight with the min version, but it's only possible once the infrastructure for such a thing is in place. CC @glandium |
Based on the discussion here, I get the impression that, while nobody is really happy with the current LLVM situation, there is agreement that we can drop support for LLVM 5, so I went ahead and opened a PR for that at #56642. I believe this should at least unblock further work on allocators. |
FWIW, it won't. Unpatched llvm version 6 still has the problem from issue #52694, and the version of llvm 6 in Ubuntu before 18.10 isn't patched for that issue (the one in Ubuntu 18.10 is, though). |
Bump minimum required LLVM version to 6.0 Based on the discussion in #55842, while the overall position of Rust wrt LLVM continues to be contentious, there does seem to be a consensus that there is no need for continued support of LLVM 5. This PR bumps our version requirement to LLVM 6.0 and makes Travis run against that. I hope that this is going to unblock #52694. If I understand correctly, while this issue still exists in LLVM 6, Ubuntu has backported the relevant patch. r? @alexcrichton
Bump minimum required LLVM version to 6.0 Based on the discussion in #55842, while the overall position of Rust wrt LLVM continues to be contentious, there does seem to be a consensus that there is no need for continued support of LLVM 5. This PR bumps our version requirement to LLVM 6.0 and makes Travis run against that. I hope that this is going to unblock #52694. If I understand correctly, while this issue still exists in LLVM 6, Ubuntu has backported the relevant patch. r? @alexcrichton
This issue, as titled, is done now in #56642, so I'm going to close it. If there's follow-up work though it'd be good to open new issues! |
It's that time of the year again - almost 6 months since we bumped the minimum supported system LLVM to version 5 (#51899). Now that emscripten has been updated to LVM 6.0.1 (#55626 - thank you @nikic ) I wanted to bump the minimum supported version to 6.
However, there has been some controversy about the minimum supported LLVM version lagging too much behind the bundled one, resulting in a lot of frustration for both contributors, users, and packagers (https://internals.rust-lang.org/t/rust-vs-llvm/8604).
Supporting older versions is particularly problematic because:
bugs!
on older LLVM versions when using some language features, at worst it silently compiles with incorrect codegen - distro packagers don't see tests failing because we/them silence them.I don't know how aware are distro packagers of these issues. I know that @cuviper and @infinity0 are aware of them, e.g., when packaging for a distro often the testsuite has to "pass" but it only passes with system LLVMs because we basically silence all tests that do not pass.
That is, currently, even though we let Rust build silently with LLVM 5, many parts of the language / compiler are just unusable (SIMD, wasm, ...). This is currently also the case for LLVM 6, and even LLVM 7!
So I think it is worth to reconsider our stance on this. I personally think that we should come up with an enforceable policy about the minimum system LLVM version that we support, and that the policy has to be better than "the last 3 LLVM releases" to add any value. If we were to continuously support the last released LLVM version (version 7 right now), we could, slowly:
Currently, because we support the last 4 LLVM releases, an upstreamed patch lets us remove code in rustc in ~2 - 2.5 years. If we were to only support the last LLVM version, that would be reduced to 6 months, making it significantly more profitable to upstream our patches. It might also make sense to keep workarounds around to keep things going if we know these only have to live for 6 months.
Also, it is unclear to me what's the value of shipping new Rust versions on distros using a 2 year old LLVM. Couldn't these distros just ship an older Rust version that's known to work with the distros LLVM version ? (by known I mean, where most tests pass instead of being silenced).
So what I'd like to know is:
I think that moving to something like that would be first step. And once everything is working release-wise for everybody, we can consider strengthening this policy to try to keep all / most tests running for one full LLVM release, and to at least encourage workarounds in rustc for the last LLVM version.
So what do you think? In particular, what do distro packagers thing? I know that @cuviper mentioned that Fedora 28 still only has LLVM6 and that it is supported till June 2019, but by ~February 2019 Rust will be targeting LLVM9 already which is released in August 2019.
cc @infinity0 @cuviper @Keruspe @glandium @alexcrichton
The text was updated successfully, but these errors were encountered: