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

Upgrade LLVM #13513

Merged
merged 7 commits into from
Apr 19, 2014
Merged

Upgrade LLVM #13513

merged 7 commits into from
Apr 19, 2014

Conversation

alexcrichton
Copy link
Member

This is a bit of an interesting upgrade to LLVM. Upstream LLVM has started using C++11 features, so they require a C++11 compiler to build. I've updated all the bots to have a C++11 compiler, and they appear to be building LLVM successfully:

  • Linux bots - I added gcc/g++ 4.7 (good enough)
  • Android bots - same as the linux ones
  • Mac bots - I installed the most recent command line tools for Lion which gives us clang 3.2, but LLVM wouldn't build unless it was explicitly asked to link to libc++ instead of libstdc++. This involved tweaking mklldeps.py and the configure script to get things to work out
  • Windows bots - mingw-w64 has gcc 4.8.1 which is sufficient for building LLVM (hurray!)
  • BSD bots - I updated FreeBSD to 10.0 which brought with it a relevant version of clang.

The largest fallout I've seen so far is that the test suite doesn't work at all on FreeBSD 10. We've already stopped gating on FreeBSD due to #13427 (we used to be on freebsd 9), so I don't think this puts us in too bad of a situation. I will continue to attempt to fix FreeBSD and the breakage on there.

The LLVM update brings with it all of the recently upstreamed LLVM patches. We only have one local patch now which is just an optimization, and isn't required to use upstream LLVM. I want to maintain compatibility with LLVM 3.3 and 3.4 while we can, and this upgrade is keeping us up to date with the 3.5 release. Once 3.5 is release we will in theory no longer require a bundled LLVM.

@alexcrichton
Copy link
Member Author

I realized that I can test locally against LLVM snapshots to ensure that we still work with 3.3/3.4, so I'm going to hold off r=thestinger'ing the last commit until I can verify that it does indeed fix 3.3/3.4

@alexcrichton
Copy link
Member Author

removing r+, @brson has worries about the linux-snap builder which I forgot to update. It's going to be a little tricky.

This comes with a number of fixes to be compatible with upstream LLVM:

* Previously all monomorphizations of "mem::size_of()" would receive the same
  symbol. In the past LLVM would silently rename duplicated symbols, but it
  appears to now be dropping the duplicate symbols and functions now. The symbol
  names of monomorphized functions are now no longer solely based on the type of
  the function, but rather the type and the unique hash for the
  monomorphization.

* Split stacks are no longer a global feature controlled by a flag in LLVM.
  Instead, they are opt-in on a per-function basis through a function attribute.
  The rust #[no_split_stack] attribute will disable this, otherwise all
  functions have #[split_stack] attached to them.

* The compare and swap instruction now takes two atomic orderings, one for the
  successful case and one for the failure case. LLVM internally has an
  implementation of calculating the appropriate failure ordering given a
  particular success ordering (previously only a success ordering was
  specified), and I copied that into the intrinsic translation so the failure
  ordering isn't supplied on a source level for now.

* Minor tweaks to LLVM's API in terms of debuginfo, naming, c++11 conventions,
  etc.
When clang is enabled, also pass through --enable-libcpp to LLVM's configure
command line to help it pick up the most recent c++ runtime library. This also
changes the mklldeps.py script to pick up on whether LLVM was linked against
stdc++ or c++ based on the --cxxflags that llvm-config prints.

In an ongoing attempt to update LLVM, the bots need to update their C compilers
to something that supports c++11 (LLVM recently switched). The OSX bots are
running Lion (10.7), which only supports up to gcc 4.2 and clang 3.2. Apparently
the libstdc++ is too old (even on the most updated command line tools) for LLVM,
but using libc++ instead appears to work just fine.
OSX often has a more recent version of clang than it does for GCC. When an older
version of gcc is detected on OSX, the --enable-clang flag is implicitly
enabled.
Older version of LLVM did not have this flag, so we need to fall back to our
previous library detection when using older versions of LLVM.
The goal of the snapshot bots is to produce binaries which can run in as many
locations as possible. Currently we build on Centos 6 for this reason, but with
LLVM's update to C++11, this reduces the number of platforms that we could
possibly run on.

This adds a --enable-llvm-static-stdcpp option to the ./configure script for
Rust which will enable building a librustc with a static dependence on
libstdc++. This normally isn't necessary, but this option can be used on the
snapshot builders in order to continue to make binaries which should be able to
run in as many locations as possible.
If a linker finds both a static and a dynamic version of the same library, then
the linker often chooses the dynamic version. This is surprising when a native
library is specified as being "static" in rust source. This modifies the linker
command line to obey the hints given in rust source files and instructing the
linker to prefer a particular version of a found library.

Unfortunately, this patch has no effect on osx because the linker supports
no such hint, and it also has no effect on windows because the linker apparently
just ignores it. For now this is predominately used to enable the previous patch
of linking to libstdc++ statically, but more support would need to be added for
this in the future if we wanted to officially support it.

cc rust-lang#12557 (doesn't close because it doesn't support OSX and windows)
@alexcrichton
Copy link
Member Author

ping @michaelwoerister

These seem to be failing because rbreak zzz isn't working in GDB. I have 7.6.1 installed locally (which works), but the bots have gdb 7.4 installed locally (which apparently doesn't work). For example:

My gdb:

$ ./x86_64-unknown-linux-gnu/stage2/bin/rustc -g ./src/test/debug-info/simple-struct.rs 
$ gdb ./simple-struct              
GNU gdb (GDB) 7.6.1-ubuntu
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/alex/code/rust4/simple-struct...done.
(gdb) rbreak zzz
Breakpoint 1 at 0x404460: file src/test/debug-info/simple-struct.rs, line 182.
static void simple-struct::zzz();
(gdb) 

gdb on the bots

$ ./x86_64-unknown-linux-gnu/stage2/bin/rustc ../src/test/debug-info/simple-struct.rs -g
$ gdb ./simple-struct 
GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2) 7.4-2012.04
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://bugs.launchpad.net/gdb-linaro/>...
Reading symbols from /home/ubuntu/src/rust-buildbot/slave/try-linux/build/obj/simple-struct...done.
(gdb) rbreak zzz
(gdb) 

There was a change to debug info during this upgrade, the LLVMDIBuilderCreateLexicalBlock function now takes a Discriminator argument at the end. I saw in LLVM they set this to 0 in a few places, so I set this to 0 always. There may have been other debuginfo changes throughout this upgrade which messed with this.

Basically, does this look like a familiar issue or something similar?

@vadimcn vadimcn mentioned this pull request Apr 18, 2014
@alexcrichton
Copy link
Member Author

Hm, it appears that a discriminator doesn't have much to do with what appears to be the problem.

It just appears that debug info isn't working at all with gdb 7.4 after this upgrade...

@michaelwoerister
Copy link
Member

I haven't seen anything like this before. I will take a look at it next
Wednesday.

On 18.04.2014 09:40, Alex Crichton wrote:

Hm, it appears that a discriminator doesn't have much to do with what
appears to be the problem.

It just appears that debug info isn't working at all with gdb 7.4
after this upgrade...

Reply to this email directly or view it on GitHub [1].

Links:

[1] #13513 (comment)

@alexcrichton
Copy link
Member Author

Hm, interesting development:

fn main() {
    zzz();
}
fn zzz() {()}
fn main() {
    zzz();
}
fn zzz() {}

In gdb 7.4, rbreak zzz woks on the second example, but not the first. Sadly changing all the debug-info tests only fixes 3 tests... (79 still failing)

@alexcrichton
Copy link
Member Author

False alarms all around! Looks like LLVM is just emitting newer dwarf. Compiling with -C llvm-args=-dwarf-version=3 fixes all tests.

This is a consequence of rust-lang#13611 and our bots running a "fairly old" gdb which
doesn't understand the newer versions of dwarf.
bors added a commit that referenced this pull request Apr 19, 2014
This is a bit of an interesting upgrade to LLVM. Upstream LLVM has started using C++11 features, so they require a C++11 compiler to build. I've updated all the bots to have a C++11 compiler, and they appear to be building LLVM successfully:

* Linux bots - I added gcc/g++ 4.7 (good enough)
* Android bots - same as the linux ones
* Mac bots - I installed the most recent command line tools for Lion which gives us clang 3.2, but LLVM wouldn't build unless it was explicitly asked to link to `libc++` instead of `libstdc++`. This involved tweaking `mklldeps.py` and the `configure` script to get things to work out
* Windows bots - mingw-w64 has gcc 4.8.1 which is sufficient for building LLVM (hurray!)
* BSD bots - I updated FreeBSD to 10.0 which brought with it a relevant version of clang.

The largest fallout I've seen so far is that the test suite doesn't work at all on FreeBSD 10. We've already stopped gating on FreeBSD due to #13427 (we used to be on freebsd 9), so I don't think this puts us in too bad of a situation. I will continue to attempt to fix FreeBSD and the breakage on there.

The LLVM update brings with it all of the recently upstreamed LLVM patches. We only have one local patch now which is just an optimization, and isn't required to use upstream LLVM. I want to maintain compatibility with LLVM 3.3 and 3.4 while we can, and this upgrade is keeping us up to date with the 3.5 release. Once 3.5 is release we will in theory no longer require a bundled LLVM.
@bors bors closed this Apr 19, 2014
@bors bors merged commit 426d701 into rust-lang:master Apr 19, 2014
@alexcrichton alexcrichton deleted the up-llvm branch April 19, 2014 03:38
Dylan-DPC pushed a commit to Dylan-DPC/rust that referenced this pull request Nov 1, 2022
…eykril

Properly handle vscode workspace changes
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 18, 2024
Style: do not defensively use `saturating_sub()`

Using `saturating_sub()` here in code which cannot fail brings a false sense of security. If for any reason a logic error was introduced and caused `self.loop_depth` to reach 0 before being decremented, using `saturating_sub(1)` would silently mask the programming error instead of panicking loudly as it should (at least in dev profile).

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants