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

rustc: Upgrade to LLVM 6 #47828

Merged
merged 1 commit into from
Feb 10, 2018
Merged

rustc: Upgrade to LLVM 6 #47828

merged 1 commit into from
Feb 10, 2018

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Jan 28, 2018

The following submodules have been updated for a new version of LLVM:

  • src/llvm
  • src/libcompiler_builtins - transitively contains compiler-rt
  • src/dlmalloc

This also updates the docker container for dist-i686-freebsd as the old 16.04
container is no longer capable of building LLVM. The
compiler-rt/compiler-builtins and dlmalloc updates are pretty routine without
much interesting happening, but the LLVM update here is of particular note.
Unlike previous updates I haven't cherry-picked all existing patches we had on
top of our LLVM branch as we have a huge amount and have at this
point forgotten what most of them are for. Instead I started from the current
release_60 branch in LLVM and only applied patches that were necessary to get
our tests working and building.

The current set of custom rustc-specific patches included in this LLVM update are:

  • rust-lang/llvm@1187443 - this is how we actually implement
    cfg(target_feature) for now and continues to not be upstreamed. While a
    hazard for SIMD stabilization this commit is otherwise keeping the status
    quo of a small rustc-specific feature.
  • rust-lang/llvm@013f2ec - this is a rustc-specific optimization that we haven't
    upstreamed, notably teaching LLVM about our allocation-related routines (which
    aren't malloc/free). Once we stabilize the global allocator routines we will
    likely want to upstream this patch, but for now it seems reasonable to keep it
    on our fork.
  • rust-lang/llvm@a65bbfd - I found this necessary to fix compilation of LLVM in
    our 32-bit linux container. I'm not really sure why it's necessary but my
    guess is that it's because of the absolutely ancient glibc that we're using.
    In any case it's only updating pieces we're not actually using in LLVM so I'm
    hoping it'll turn out alright. This doesn't seem like something we'll want to
    upstream.c
  • rust-lang/llvm@77ab1f0 - this is what's actually enabling LLVM to build in our
    i686-freebsd container, I'm not really sure what's going on but we for sure
    probably don't want to upstream this and otherwise it seems not too bad for
    now at least.
  • rust-lang/llvm@9eb9267 - we currently suffer on MSVC from an upstream bug
    which although diagnosed to a particular revision isn't currently fixed
    upstream (and the bug itself doesn't seem too active). This commit is a
    partial revert of the suspected cause of this regression (found via a
    bisection). I'm sort of hoping that this eventually gets fixed upstream with a
    similar fix (which we can replace in our branch), but for now I'm also hoping
    it's a relatively harmless change to have.

After applying these patches (plus one backport which should be backported
upstream
) I believe we should have all tests working on all
platforms in our current test suite. I'm like 99% sure that we'll need some more
backports as issues are reported for LLVM 6 when this propagates through
nightlies, but that's sort of just par for the course nowadays!

In any case though some extra scrutiny of the patches here would definitely be
welcome, along with scrutiny of the "missing patches" like a change to pass
manager order
, another change to pass manager
order
, some compile fixes for
sparc
, and some fixes for
solaris
.


The update to LLVM 6 is desirable for a number of reasons, notably:

  • This'll allow us to keep up with the upstream wasm backend, picking up new
    features as they start landing.
  • Upstream LLVM has fixed a number of SIMD-related compilation errors,
    especially around AVX-512 and such.
  • There's a few assorted known bugs which are fixed in LLVM 5 and aren't fixed
    in the LLVM 4 branch we're using.
  • Overall it's not a great idea to stagnate with our codegen backend!

This update is mostly powered by #47730 which is allowing us to update LLVM
independent of the version of LLVM that Emscripten is locked to. This means
that when compiling code for Emscripten we'll still be using the old LLVM 4
backend, but when compiling code for any other target we'll be using the new
LLVM 6 target. Once Emscripten updates we may no longer need this distinction,
but we're not sure when that will happen!

Closes #43370
Closes #43418
Closes #47015
Closes #47683
Closes rust-lang/stdarch#157
Closes rust-lang-nursery/rust-wasm#3

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

Note that this is currently based on #47730 which I'll rebase out once that lands. Also @rust-lang/infra could this get a perf run and a crater run? (to hopefully head of anything massive!)

@bors: try

cc @rust-lang/compiler

@bors
Copy link
Contributor

bors commented Jan 28, 2018

⌛ Trying commit d9031ae with merge 38bd381...

bors added a commit that referenced this pull request Jan 28, 2018
rustc: Upgrade to LLVM 6

The following submodules have been updated for a new version of LLVM:

- `src/llvm`
- `src/libcompiler_builtins` - transitively contains compiler-rt
- `src/dlmalloc`

This also updates the docker container for dist-i686-freebsd as the old 16.04
container is no longer capable of building LLVM. The
compiler-rt/compiler-builtins and dlmalloc updates are pretty routine without
much interesting happening, but the LLVM update here is of particular note.
Unlike previous updates I haven't cherry-picked all existing patches we had on
top of our LLVM branch as we have a [huge amount][patches4] and have at this
point forgotten what most of them are for. Instead I started from the current
`release_60` branch in LLVM and only applied patches that were necessary to get
our tests working and building.

The current set of custom rustc-specific patches included in this LLVM update are:

* rust-lang/llvm@1187443 - this is how we actually implement
  `cfg(target_feature)` for now and continues to not be upstreamed. While a
  hazard for SIMD stabilization this commit is otherwise keeping the status
  quo of a small rustc-specific feature.
* rust-lang/llvm@013f2ec - this is a rustc-specific optimization that we haven't
  upstreamed, notably teaching LLVM about our allocation-related routines (which
  aren't malloc/free). Once we stabilize the global allocator routines we will
  likely want to upstream this patch, but for now it seems reasonable to keep it
  on our fork.
* rust-lang/llvm@a65bbfd - I found this necessary to fix compilation of LLVM in
  our 32-bit linux container. I'm not really sure why it's necessary but my
  guess is that it's because of the absolutely ancient glibc that we're using.
  In any case it's only updating pieces we're not actually using in LLVM so I'm
  hoping it'll turn out alright. This doesn't seem like something we'll want to
  upstream.c
* rust-lang/llvm@77ab1f0 - this is what's actually enabling LLVM to build in our
  i686-freebsd container, I'm not really sure what's going on but we for sure
  probably don't want to upstream this and otherwise it seems not too bad for
  now at least.
* rust-lang/llvm@9eb9267 - we currently suffer on MSVC from an [upstream bug]
  which although diagnosed to a particular revision isn't currently fixed
  upstream (and the bug itself doesn't seem too active). This commit is a
  partial revert of the suspected cause of this regression (found via a
  bisection). I'm sort of hoping that this eventually gets fixed upstream with a
  similar fix (which we can replace in our branch), but for now I'm also hoping
  it's a relatively harmless change to have.

After applying these patches (plus one [backport] which should be [backported
upstream][llvm-back]) I believe we should have all tests working on all
platforms in our current test suite. I'm like 99% sure that we'll need some more
backports as issues are reported for LLVM 6 when this propagates through
nightlies, but that's sort of just par for the course nowadays!

In any case though some extra scrutiny of the patches here would definitely be
welcome, along with scrutiny of the "missing patches" like a [change to pass
manager order](rust-lang/llvm@2717444), [another change to pass manager
order](rust-lang/llvm@c782feb), some [compile fixes for
sparc](rust-lang/llvm@1a83de6), and some [fixes for
solaris](rust-lang/llvm@c2bfe0a).

[patches4]: rust-lang/llvm@5401fdf...rust-llvm-release-4-0-1
[backport]: rust-lang/llvm@5c54c25
[llvm-back]: https://bugs.llvm.org/show_bug.cgi?id=36114
[upstream bug]: https://bugs.llvm.org/show_bug.cgi?id=36096

---

The update to LLVM 6 is desirable for a number of reasons, notably:

* This'll allow us to keep up with the upstream wasm backend, picking up new
  features as they start landing.
* Upstream LLVM has fixed a number of SIMD-related compilation errors,
  especially around AVX-512 and such.
* There's a few assorted known bugs which are fixed in LLVM 5 and aren't fixed
  in the LLVM 4 branch we're using.
* Overall it's not a great idea to stagnate with our codegen backend!

This update is mostly powered by #47730 which is allowing us to update LLVM
*independent* of the version of LLVM that Emscripten is locked to. This means
that when compiling code for Emscripten we'll still be using the old LLVM 4
backend, but when compiling code for any other target we'll be using the new
LLVM 6 target. Once Emscripten updates we may no longer need this distinction,
but we're not sure when that will happen!

Closes #43370
Closes #43418
Closes #47015
Closes #47683
Closes rust-lang/stdarch#157
Closes rust-lang-nursery/rust-wasm#3
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 28, 2018
@bors
Copy link
Contributor

bors commented Jan 28, 2018

☀️ Test successful - status-travis
State: approved= try=True

@shepmaster shepmaster added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jan 29, 2018
@alexcrichton
Copy link
Member Author

Perf comparison shows that nothing is too bad!

@kennytm kennytm removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 29, 2018
@est31
Copy link
Member

est31 commented Jan 29, 2018

Up to 13.5% improvement... wow.

@nikomatsakis
Copy link
Contributor

@bors r+

Let's do it.

@bors
Copy link
Contributor

bors commented Jan 29, 2018

📌 Commit 586488c has been approved by nikomatsakis

@cuviper
Copy link
Member

cuviper commented Jan 29, 2018

rust-lang/llvm@1187443 - this is how we actually implement
cfg(target_feature) for now and continues to not be upstreamed. While a
hazard for SIMD stabilization this commit is otherwise keeping the status
quo of a small rustc-specific feature.

Maybe update run-pass/sse2 to // min-llvm-version 6.0? But really that needs a rustllvm flag.

Apart from that, testing on this branch also looks good with external LLVM 5.

@mattico
Copy link
Contributor

mattico commented Jan 30, 2018

I did some investigation on the FreeBSD issue. FreeBSD 10 uses Clang 3.0. Clang didn't support std::atomic until 3.3. I think that's all the issue is. FreeBSD 10 is only supported until Oct 31 of this year, so it should be safe to update that soon enough.

@alexcrichton
Copy link
Member Author

@cuviper ah yes that test should definitely have a // no-system-llvm flag. Otherwise though this is an existing issue, I just wanted to highlight it again :)

@mattico ok cool, thanks!

@kennytm kennytm 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-crater Status: Waiting on a crater run to be completed. labels Jan 30, 2018
@kennytm
Copy link
Member

kennytm commented Jan 30, 2018

@bors p=1

@bors
Copy link
Contributor

bors commented Jan 30, 2018

⌛ Testing commit 586488c with merge 22d405f...

bors added a commit that referenced this pull request Jan 30, 2018
rustc: Upgrade to LLVM 6

The following submodules have been updated for a new version of LLVM:

- `src/llvm`
- `src/libcompiler_builtins` - transitively contains compiler-rt
- `src/dlmalloc`

This also updates the docker container for dist-i686-freebsd as the old 16.04
container is no longer capable of building LLVM. The
compiler-rt/compiler-builtins and dlmalloc updates are pretty routine without
much interesting happening, but the LLVM update here is of particular note.
Unlike previous updates I haven't cherry-picked all existing patches we had on
top of our LLVM branch as we have a [huge amount][patches4] and have at this
point forgotten what most of them are for. Instead I started from the current
`release_60` branch in LLVM and only applied patches that were necessary to get
our tests working and building.

The [current set of custom rustc-specific patches](rust-lang/llvm@f128612...rust-llvm-release-6-0-0) included in this LLVM update are:

* rust-lang/llvm@1187443 - this is how we actually implement
  `cfg(target_feature)` for now and continues to not be upstreamed. While a
  hazard for SIMD stabilization this commit is otherwise keeping the status
  quo of a small rustc-specific feature.
* rust-lang/llvm@013f2ec - this is a rustc-specific optimization that we haven't
  upstreamed, notably teaching LLVM about our allocation-related routines (which
  aren't malloc/free). Once we stabilize the global allocator routines we will
  likely want to upstream this patch, but for now it seems reasonable to keep it
  on our fork.
* rust-lang/llvm@a65bbfd - I found this necessary to fix compilation of LLVM in
  our 32-bit linux container. I'm not really sure why it's necessary but my
  guess is that it's because of the absolutely ancient glibc that we're using.
  In any case it's only updating pieces we're not actually using in LLVM so I'm
  hoping it'll turn out alright. This doesn't seem like something we'll want to
  upstream.c
* rust-lang/llvm@77ab1f0 - this is what's actually enabling LLVM to build in our
  i686-freebsd container, I'm not really sure what's going on but we for sure
  probably don't want to upstream this and otherwise it seems not too bad for
  now at least.
* rust-lang/llvm@9eb9267 - we currently suffer on MSVC from an [upstream bug]
  which although diagnosed to a particular revision isn't currently fixed
  upstream (and the bug itself doesn't seem too active). This commit is a
  partial revert of the suspected cause of this regression (found via a
  bisection). I'm sort of hoping that this eventually gets fixed upstream with a
  similar fix (which we can replace in our branch), but for now I'm also hoping
  it's a relatively harmless change to have.

After applying these patches (plus one [backport] which should be [backported
upstream][llvm-back]) I believe we should have all tests working on all
platforms in our current test suite. I'm like 99% sure that we'll need some more
backports as issues are reported for LLVM 6 when this propagates through
nightlies, but that's sort of just par for the course nowadays!

In any case though some extra scrutiny of the patches here would definitely be
welcome, along with scrutiny of the "missing patches" like a [change to pass
manager order](rust-lang/llvm@2717444), [another change to pass manager
order](rust-lang/llvm@c782feb), some [compile fixes for
sparc](rust-lang/llvm@1a83de6), and some [fixes for
solaris](rust-lang/llvm@c2bfe0a).

[patches4]: rust-lang/llvm@5401fdf...rust-llvm-release-4-0-1
[backport]: rust-lang/llvm@5c54c25
[llvm-back]: https://bugs.llvm.org/show_bug.cgi?id=36114
[upstream bug]: https://bugs.llvm.org/show_bug.cgi?id=36096

---

The update to LLVM 6 is desirable for a number of reasons, notably:

* This'll allow us to keep up with the upstream wasm backend, picking up new
  features as they start landing.
* Upstream LLVM has fixed a number of SIMD-related compilation errors,
  especially around AVX-512 and such.
* There's a few assorted known bugs which are fixed in LLVM 5 and aren't fixed
  in the LLVM 4 branch we're using.
* Overall it's not a great idea to stagnate with our codegen backend!

This update is mostly powered by #47730 which is allowing us to update LLVM
*independent* of the version of LLVM that Emscripten is locked to. This means
that when compiling code for Emscripten we'll still be using the old LLVM 4
backend, but when compiling code for any other target we'll be using the new
LLVM 6 target. Once Emscripten updates we may no longer need this distinction,
but we're not sure when that will happen!

Closes #43370
Closes #43418
Closes #47015
Closes #47683
Closes rust-lang/stdarch#157
Closes rust-lang-nursery/rust-wasm#3
@bors
Copy link
Contributor

bors commented Jan 30, 2018

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Jan 30, 2018

😕

Assuming spurious first.

@bors retry #33434

Could not compile rustc_errors with error code 3221225477 (0xc0000005)

[01:40:22] Building stage1 compiler artifacts (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc)
[01:40:22]    Compiling graphviz v0.0.0 (file:///C:/projects/rust/src/libgraphviz)
<snip>
[01:41:43]    Compiling rustc_errors v0.0.0 (file:///C:/projects/rust/src/librustc_errors)
[01:41:49] error: Could not compile `rustc_errors`.
[01:41:49] 
[01:41:49] Caused by:
[01:41:49]   process didn't exit successfully: `C:\projects\rust\build\bootstrap/debug/rustc --crate-name rustc_errors librustc_errors\lib.rs --color always --error-format json --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=65687244e9f663b1 -C extra-filename=-65687244e9f663b1 --out-dir C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps --target x86_64-pc-windows-msvc -L dependency=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps -L dependency=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\release\deps --extern rustc_data_structures=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps\rustc_data_structures-fe1c5452c3c89758.dll --extern serialize=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps\serialize-e9133ab5aa0e0076.dll --extern serialize=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps\libserialize-e9133ab5aa0e0076.rlib --extern unicode_width=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps\libunicode_width-850dabec54650530.rlib --extern syntax_pos=C:\projects\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\deps\syntax_pos-f4ba1a45fc002844.dll` (exit code: 3221225477)
[01:41:49] thread 'main' panicked at 'command did not execute successfully: "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage0/bin\\cargo.exe" "build" "--target" "x86_64-pc-windows-msvc" "-j" "2" "--release" "--locked" "--color" "always" "--features" "" "--manifest-path" "C:\\projects\\rust\\src/rustc/Cargo.toml" "--message-format" "json"
[01:41:49] expected success, got: exit code: 101', bootstrap\compile.rs:1062:9
[01:41:49] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:41:49] failed to run: C:\projects\rust\build\bootstrap\debug\bootstrap dist

@alexcrichton
Copy link
Member Author

@bors: r-

nah I saw that in development and thought it was spurious, at this point I sort of doubt it is...

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 30, 2018
@Zoxc
Copy link
Contributor

Zoxc commented Jan 30, 2018

I see access violations all the time when building on Windows using master, rebuilding makes it go away, so this error might be spurious.

@alexcrichton
Copy link
Member Author

@kennytm I'm sort of just glad to have this done with, but I'll send a follow-up to clean up that code. With #47657 (comment) though it seems like it's not helping anyway...

@bluss bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 10, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 10, 2018
Introduced in rust-lang#47828 to help track down some bugs, it landed a bit hastily so
this is intended on cleaning it up a bit.
@shepmaster
Copy link
Member

With #47657 (comment) though it seems like it's not helping anyway...

I think you meant to refer to #48116?

bors bot added a commit to phil-opp/blog_os that referenced this pull request Feb 10, 2018
385: First bits of the second edition r=phil-opp a=phil-opp

This PR adds the first two posts for the second edition, “A Freestanding Rust Binary” and “A Minimal Rust Kernel”. The largest changes in comparison to the first edition are:

- Instead of GRUB, we use our own [bootloader](https://github.com/rust-osdev/bootloader) (written in Rust) and our [bootimage](https://github.com/rust-osdev/bootimage) tool. This removes the dependencies on GRUB and `nasm`. Note that both tools are still experimental and might contain bugs.
- Support for Windows and Mac: Without GRUB, there's nothing preventing us from building on Windows or Mac OS anymore! We added additional CI jobs at travis and appveyor to ensure that the project always builds on all three platforms. (At the moment, users still need to install LLD, but with the [LLVM 6 update of rustc](rust-lang/rust#47828) we should have a builtin LLD soon.)
- No assembly in the main posts. Instead, we're creating a `no_std` _executable_ and relying on our custom bootloader to perform the initial page mapping and the switch to long mode.
- No linker script: Our bootloader loads the kernel in a clean address space, so we can just use the default executable layout and don't need a linker script. (This also means that users that want a higher half kernel just need to update the mapping in their linker script. However, I'm not sure if it's a good idea to add this to the blog. Maybe later, when we begin to run user programs.)

These changes only land in “beta mode” with this PR, which means that they're not linked from the front page yet. We will do that in a follow up PR.
kennytm added a commit to kennytm/rust that referenced this pull request Feb 10, 2018
…kennytm

Explain unusual debugging code in librustc

Introduced in rust-lang#47828 to help track down some bugs, it landed a bit hastily so
this is intended on cleaning it up a bit.
@alexcrichton
Copy link
Member Author

@shepmaster oh I was just linking to the failure on #47657, but yeah #48116 is in general tracking the failure.

@boozook
Copy link

boozook commented Feb 14, 2018

Now really looking forward to the bitcode feature of LLVM7. Today - LLVM6, tomorrow - LLVM7!

@martell
Copy link
Contributor

martell commented Feb 15, 2018

@alexcrichton
Author of https://reviews.llvm.org/D29892 here

rust-lang/llvm@9eb9267 - we currently suffer on MSVC from an upstream bug which although diagnosed to a particular revision isn't currently fixed upstream (and the bug itself doesn't seem too active). This commit is a partial revert of the suspected cause of this regression (found via a bisection). I'm sort of hoping that this eventually gets fixed upstream with a similar fix (which we can replace in our branch), but for now I'm also hoping it's a relatively harmless change to have.

https://bugs.llvm.org/show_bug.cgi?id=36096
Let's make this bug report a little more active and see if we can find the correct fix. :)

@gaming-hacker
Copy link

Not to nag but has this issue with llvm-60 been resolved on OSX/Linux and merged into master?

@shepmaster
Copy link
Member

@gaming-hacker

Yes it has been merged. You can tell by scrolling to the top of the page and checking out the header:

image

If you click through to the commit, you can see which tags the commit is part of:

image

You can also check out the 1.25 release notes which lists this (emphasis mine):

The last few releases have been relatively minor, but Rust 1.25 contains a bunch of stuff! The first one is straightforward: we’ve upgraded to LLVM 6 from LLVM 4

@gaming-hacker
Copy link

Ah thanks, i use source tree as a gui to display the status of the commits and it shows these as unmerged, i'll report a bug to source tree.

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet