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

Bump minimal supported LLVM version to 9 #78848

Merged
merged 7 commits into from
Nov 15, 2020
Merged

Conversation

DevJPM
Copy link
Contributor

@DevJPM DevJPM commented Nov 7, 2020

This bumps the minimal tested llvm version to 9.
This should enable supporting newer LLVM features (and CPU extensions).

This was motived by #78361 having to drop features because of LLVM 8 not supporting certain CPU extensions yet.
This was declared relatively uncontroversial on Zulip.

Paging @eddyb because there was a comment in the dockerfile describing a hack (which I don't quite understand) which was also blocked by not having LLVM 9.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pietroalbini (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 7, 2020
@DevJPM DevJPM changed the title Bumped minimal tested LLVM version to 9 Bump minimal tested LLVM version to 9 Nov 7, 2020
@nikic
Copy link
Contributor

nikic commented Nov 7, 2020

This builder always tests the lowest supported version of LLVM -- if you want to bump the tested version, you actually need to drop support for LLVM 8.

In particular you need to adjust

fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) {
, and then grep for LLVM_VERSION_GE, LLVM_VERSION_LT, get_major_version, min-llvm-version to find places where compatibility code can be dropped.

@DevJPM DevJPM changed the title Bump minimal tested LLVM version to 9 Bump supported LLVM version to 9 Nov 7, 2020
@DevJPM DevJPM changed the title Bump supported LLVM version to 9 Bump minimal supported LLVM version to 9 Nov 7, 2020
@DevJPM
Copy link
Contributor Author

DevJPM commented Nov 7, 2020

@nikic Thank you very much for these suggestions. They have been exactly what I've been looking for - but failed to find amidst the size of Rust.

The latest commit does indeed grep for these constants and drops compatibility code whereever possible and sensible.

@pietroalbini
Copy link
Member

The CI changes look good! r? @nikic for the rest.

@rust-highfive rust-highfive assigned nikic and unassigned pietroalbini Nov 9, 2020
@cuviper cuviper added the relnotes Marks issues that should be documented in the release notes of the next release. label Nov 9, 2020
if feature == new {
return old;
}
for &(old, new) in LLVM9_FEATURE_CHANGES {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay to just drop this code entirely. In later LLVM upgrades we decided that we should not do bidirectional emulation (there are no stability guarantees for custom target definitions).

@nikic
Copy link
Contributor

nikic commented Nov 10, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Nov 10, 2020

📌 Commit 41387913d38dec9de4e7ec48498357cb432ffcd3 has been approved by nikic

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 10, 2020
@DevJPM
Copy link
Contributor Author

DevJPM commented Nov 12, 2020

Sorry to anyone who might have been notified by my failed attempt at fixing the merge conflict indicated by GitHub

This bumps the minimal tested llvm version to 9.
This should enable supporting newer LLVM features (and CPU extensions).
apparently llvm-8-tools already had llvm-8-dev as a dependency
which was removed in llvm-9-tools, so we need to explicitly pull
llvm-9-dev to make a build
This commit grepped for LLVM_VERSION_GE, LLVM_VERSION_LT, get_major_version and
min-llvm-version and statically evaluated every expression possible
(and sensible) assuming that the LLVM version is >=9 now
…ersion

The function was only used in LLVM 8 compatibility code
and was found and flagged by dead code detection and now removed.
as requested in the review and argued that this is only consistent with later LLVM upgrades
@DevJPM
Copy link
Contributor Author

DevJPM commented Nov 12, 2020

The update was due to a merge conflict in attribute.rs where my changes forced a re-format to one-line and another change removed one intermediate value

@DevJPM
Copy link
Contributor Author

DevJPM commented Nov 14, 2020

So, it appears that after the approval of this PR a merge conflict got merged into master - which I fixed - but now appears that bors has no intention of merging this PR anymore without another approval? (Hoping that I read this page correctly)

So, can I ask for an approval of the conflict-free PR? @nikic ? (Hoping that I understand the workflows around here correctly)

@nikic
Copy link
Contributor

nikic commented Nov 14, 2020

You got that right.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 14, 2020

📌 Commit 86193ca has been approved by nikic

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 15, 2020
Rollup of 15 pull requests

Successful merges:

 - rust-lang#78352 (Do not call `unwrap` with `signatures` option enabled)
 - rust-lang#78590 (refactor: removing alloc::collections::vec_deque ignore-tidy-filelength)
 - rust-lang#78848 (Bump minimal supported LLVM version to 9)
 - rust-lang#78856 (Explicitly checking for or-pattern before test)
 - rust-lang#78948 (test: add `()=()=()=...` to weird-exprs.rs)
 - rust-lang#78962 (Add a test for r# identifiers)
 - rust-lang#78963 (Added some unit tests as requested)
 - rust-lang#78966 (Never inline C variadics, cold functions, functions with incompatible attributes ...)
 - rust-lang#78968 (Include llvm-as in llvm-tools-preview component)
 - rust-lang#78969 (Normalize function type during validation)
 - rust-lang#78980 (Fix rustc_ast_pretty print_qpath resulting in invalid macro input)
 - rust-lang#78986 (Avoid installing external LLVM dylibs)
 - rust-lang#78988 (Fix an intrinsic invocation on threaded wasm)
 - rust-lang#78993 (rustc_target: Fix dash vs underscore mismatches in option names)
 - rust-lang#79013 (Clean up outdated `use_once_payload` pretty printer comment)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ae7020f into rust-lang:master Nov 15, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 15, 2020
DevJPM added a commit to DevJPM/rust that referenced this pull request Nov 15, 2020
With rust-lang#78848 merged, the minimum supported LLVM version is now 9
which means we can actually use the target features introduced in LLVM 9
@DevJPM DevJPM deleted the ci-llvm-9 branch November 15, 2020 10:42
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2020
Remove Hacks and Fixmes from PR CI's LLVM-9 Container

Now with LLVM 9 being the minimum supported version (thanks to rust-lang#78848 ), we can
finally remove the hacks in the dockerfile.

This wasn't in the main PR bumping the version as I didn't quite
understand what's going on and needed here.

Relevant issues and PRs:

- Issue rust-lang#69823
- PR rust-lang#70989

I hope I actually adressed things correctly here?
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Feb 14, 2021
Pkgsrc changes:
 * Adjust patches, convert tabs to spaces so that tests pass.
 * Remove patches which are no longer needed (upstream changed)
 * Minor adjustments for SunOS, e.g. disable stack probes.
 * Adjust cargo checksum patching accordingly.
 * Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway...

Upstream changes:

Version 1.49.0 (2020-12-31)
============================

Language
-----------------------

- [Unions can now implement `Drop`, and you can now have a field in a union
  with `ManuallyDrop<T>`.][77547]
- [You can now cast uninhabited enums to integers.][76199]
- [You can now bind by reference and by move in patterns.][76119] This
  allows you to selectively borrow individual components of a type. E.g.
  ```rust
  #[derive(Debug)]
  struct Person {
      name: String,
      age: u8,
  }

  let person = Person {
      name: String::from("Alice"),
      age: 20,
  };

  // `name` is moved out of person, but `age` is referenced.
  let Person { name, ref age } = person;
  println!("{} {}", name, age);
  ```

Compiler
-----------------------

- [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228]
- [Added tier 2 support for `aarch64-apple-darwin`.][75991]
- [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914]
- [Added tier 3 support for `mipsel-unknown-none`.][78676]
- [Raised the minimum supported LLVM version to LLVM 9.][78848]
- [Output from threads spawned in tests is now captured.][78227]
- [Change os and vendor values to "none" and "unknown" for some targets][78951]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
-----------------------

- [`RangeInclusive` now checks for exhaustion when calling `contains`
  and indexing.][78109]
- [`ToString::to_string` now no longer shrinks the internal buffer
  in the default implementation.][77997]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
  arrays of any length.][74989]

Stabilized APIs
---------------

- [`slice::select_nth_unstable`]
- [`slice::select_nth_unstable_by`]
- [`slice::select_nth_unstable_by_key`]

The following previously stable methods are now `const`.

- [`Poll::is_ready`]
- [`Poll::is_pending`]

Cargo
-----------------------
- [Building a crate with `cargo-package` should now be independently
  reproducible.][cargo/8864]
- [`cargo-tree` now marks proc-macro crates.][cargo/8765]
- [Added `CARGO_PRIMARY_PACKAGE` build-time environment
  variable.]  [cargo/8758] This variable will be set if the crate
  being built is one the user selected to build, either with `-p`
  or through defaults.
- [You can now use glob patterns when specifying packages &
  targets.][cargo/8752]


Compatibility Notes
-------------------
- [Demoted `i686-unknown-freebsd` from host tier 2 to target tier
  2 support.][78746]
- [Macros that end with a semi-colon are now treated as statements
  even if they expand to nothing.][78376]
- [Rustc will now check for the validity of some built-in attributes
  on enum variants.][77015] Previously such invalid or unused
  attributes could be ignored.
- Leading whitespace is stripped more uniformly in documentation
  comments, which may change behavior. You read [this post about
  the changes][rustdoc-ws-post] for more details.
- [Trait bounds are no longer inferred for associated types.][79904]

Internal Only
-------------
These changes provide no direct user facing benefits, but represent
significant improvements to the internals and overall performance
of rustc and related tools.

- [rustc's internal crates are now compiled using the `initial-exec` Thread
  Local Storage model.][78201]
- [Calculate visibilities once in resolve.][78077]
- [Added `system` to the `llvm-libunwind` bootstrap config option.][77703]
- [Added `--color` for configuring terminal color support to bootstrap.][79004]


[75991]: rust-lang/rust#75991
[78951]: rust-lang/rust#78951
[78848]: rust-lang/rust#78848
[78746]: rust-lang/rust#78746
[78376]: rust-lang/rust#78376
[78228]: rust-lang/rust#78228
[78227]: rust-lang/rust#78227
[78201]: rust-lang/rust#78201
[78109]: rust-lang/rust#78109
[78077]: rust-lang/rust#78077
[77997]: rust-lang/rust#77997
[77703]: rust-lang/rust#77703
[77547]: rust-lang/rust#77547
[77015]: rust-lang/rust#77015
[76199]: rust-lang/rust#76199
[76119]: rust-lang/rust#76119
[75914]: rust-lang/rust#75914
[74989]: rust-lang/rust#74989
[79004]: rust-lang/rust#79004
[78676]: rust-lang/rust#78676
[79904]: rust-lang/rust#79904
[cargo/8864]: rust-lang/cargo#8864
[cargo/8765]: rust-lang/cargo#8765
[cargo/8758]: rust-lang/cargo#8758
[cargo/8752]: rust-lang/cargo#8752
[`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable
[`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by
[`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key
[`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html
[`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready
[`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending
[rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants