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

Update to LLVM 9 #19

Merged
merged 10 commits into from Jul 11, 2019

Conversation

@nikic
Copy link
Collaborator

commented Jul 7, 2019

LLVM 9 will be branched soon, so it's time to work on an upgrade... This is a rebase onto bd791b5. Notes:

@nikic nikic changed the base branch from master to rustc/8.0-2019-03-18 Jul 7, 2019

@cuviper

This comment has been minimized.

Copy link
Member

commented Jul 7, 2019

This may be obvious from the conflicts, but this should be created as a new branch.

+1,850,784 −1,062,661

😲

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Thanks! I was poking at this a few days ago but I didn't try to get the LLDB patches working like you did. I'm not actually sure if we need them any more since I don't think they're actively developed and I don't think we ever got to shipping those patches anyway.

For the adding accessors patch I think that you may have an extra function left in there? I think it showed up during a rebase conflict but all we need should look like alexcrichton@78a950e.

It's also fine to drop the wasm dwarf patch, when this gets closer to landing we can notify Yury that we'll need a rebase of it.

FWIW I also went ahead and just rolled all our build-in-old-containers related patches into one commit as it's been a bit of a bummer to keep looking at a stack of multiple of them!

Otherwise I've pushed up today's master branch of LLVM (assuming you picked more-or-less an arbitrary commit to fork from as well) to https://github.com/rust-lang/llvm-project/tree/rustc/9.0-2019-07-08, and this looks good to me otherwise! Feel free to either send a PR for that branch or just push directly.

Finally, FWIW, this is as far as I got upgrading our C++. I hit LLVM assertions quickly in stage1 though so there's definitely more to work through.

bitshifter and others added some commits Jul 10, 2016

Add accessors for MCSubtargetInfo CPU and Feature tables
This is needed for `-C target-cpu=help` and `-C target-feature=help` in rustc
Fix compile on dist-i686-linux builder
If this lines are present then we apparently get errors [1] when compiling in
the current [2] dist-i686-linux container. Attempts to upgrade both gcc and
binutils did not fix the error, so it appears that this may just be a bug in the
super old glibc we're using on the dist-i686-linux container.

We don't actually need this code anyway, so just work around these issues by
removing references to the `*64` functions. This'll get things compiling
locally and shouldn't be a regression in functionality.

[1]: https://travis-ci.org/rust-lang/rust/jobs/257578199
[2]: https://github.com/rust-lang/rust/tree/eba9d7f08ce5c90549ee52337aca0010ad566f0d/src/ci/docker/dist-i686-linux
Disable checks for libatomic for now
For whatever reason this is failing the i686-freebsd builder in the Rust repo
as-of this red-hot moment. The build seems to work fine without it so let's just
remove it for now and pray there's a better fix later.

Although if you're reading this and know of a better fix, we'd love to remove
this!
Compile with /MT on MSVC
Can't seem to figure out how to do this without this patch...
Fix compile on dist-x86_64-linux builder
Apparently glibc is so old it doesn't have the _POSIX_ARG_MAX constant. This
shouldn't affect anything we use anyway though.

https://travis-ci.org/rust-lang/rust/jobs/399333071

@nikic nikic changed the base branch from rustc/8.0-2019-03-18 to rustc/9.0-2019-07-08 Jul 8, 2019

@nikic nikic force-pushed the nikic:llvm-9 branch from a8b45fe to f714d96 Jul 8, 2019

@nikic nikic referenced this pull request Jul 8, 2019

Merged

Prepare for LLVM 9 update #62474

@cuviper

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

but I didn't try to get the LLDB patches working like you did. I'm not actually sure if we need them any more since I don't think they're actively developed and I don't think we ever got to shipping those patches anyway.

I think we build the lldb component in CI, but do not yet ship it in rustup.

Finally, FWIW, this is as far as I got upgrading our C++. I hit LLVM assertions quickly in stage1 though so there's definitely more to work through.

Aside, 8.0.1 is also due soon, to which I was going to rebase here once released. If this 9.0 branch may be some time in stabilizing, should I still do 8.0.1 in the meantime?

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Indeed yeah, we never shipped in rustup because we never figured out how to best do that in terms of platform guarantees and whatnot. (I think maybe signing as well? anyway...) In any case the lack of maintainer and development for these patches means that I don't think we're obligated to keep them going, the intention was to eventually send them all upstream once they were tested anyway.

My historical experience is that LLVM upgrades take quite some time, so doing 8.0.1 in parallel seems not entirely unreasonable because it will likely finish before the 9.0.0 upgrade.

bors added a commit to rust-lang/rust that referenced this pull request Jul 8, 2019

Auto merge of #62474 - nikic:update-llvm, r=<try>
[WIP] Update to LLVM 9

Main changes:

 * In preparation for opaque pointer types, the `byval` attribute now takes a type. As such, the argument type needs to be threaded through to the function/callsite attribute application logic.
 * On ARM the `+fp-only-sp` and `+d16` features have become `-fp64` and `-d32`. I've switched the target definitions to use the new names, but also added bidirectional emulation so either can be used on any LLVM version for backwards compatibility.
 * The datalayout can now specify function pointer alignment. In particular on ARM `Fi8` is specified, which means that function pointer alignment is independent of function alignment. I've added this to our datalayouts to match LLVM (which is something we check) and strip the fnptr alignment for older LLVM versions.
 * The fmul/fadd reductions now always respect the accumulator (including for unordered reductions), so we should pass the identity instead of undef.

Open issues:

 * https://reviews.llvm.org/D62106 causes linker errors with ld.bdf due to https://sourceware.org/bugzilla/show_bug.cgi?id=24784. To avoid this I've enabled `RelaxELFRelocations`, which results in a GOTPCRELX relocation for `__tls_get_addr` and avoids the issue. However, this is likely not acceptable because relax relocations are not supported by older linker versions. We may need an LLVM option to keep using PLT for `__tls_get_addr` despite `RtLibUseGOT`.

The corresponding llvm-project PR is rust-lang/llvm-project#19.

r? @ghost
@nikic

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 8, 2019

@cuviper Are there any particular patches we want to pick up from 8.0.1? I don't think we requested backports for any of our cherry-picks.

For the record, this is the PR for LLVM 9 on the rustc side: rust-lang/rust#62474 It's currently in the "works for me" state, but will probably take a while to appease the CI gods...

@cuviper

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

@cuviper Are there any particular patches we want to pick up from 8.0.1? I don't think we requested backports for any of our cherry-picks.

Nothing in particular -- I just think it's a good idea to track the stable releases. If you manage to land an LLVM 9 snapshot first, so be it.

@nikic

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 8, 2019

Thanks! I was poking at this a few days ago but I didn't try to get the LLDB patches working like you did. I'm not actually sure if we need them any more since I don't think they're actively developed and I don't think we ever got to shipping those patches anyway.

I only did some basic rebase conflict resolution here ... I've now tried actually building with LLDB and quite predictably it fails. I've fixed the first few build failures in https://github.com/nikic/llvm-project/commits/llvm-9-lldb, but there's always more (seems like there were some larger changes to DIERef).

If the lldb patches aren't actually in use and we can drop the --enable-lldb build, that would certainly be easier...

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I think it's fine to drop --enable-lldb from the builder configuration, we've never shipped it and I don't think we're likely to any time soon. I also think it's fine to drop LLDB patches here, they're still present on the previous branches for when the work needs to be picked up again.

Centril added a commit to Centril/rust that referenced this pull request Jul 10, 2019

Rollup merge of rust-lang#62474 - nikic:update-llvm, r=alexcrichton
Prepare for LLVM 9 update

Main changes:

 * In preparation for opaque pointer types, the `byval` attribute now takes a type. As such, the argument type needs to be threaded through to the function/callsite attribute application logic.
 * On ARM the `+fp-only-sp` and `+d16` features have become `-fp64` and `-d32`. I've switched the target definitions to use the new names, but also added bidirectional emulation so either can be used on any LLVM version for backwards compatibility.
 * The datalayout can now specify function pointer alignment. In particular on ARM `Fi8` is specified, which means that function pointer alignment is independent of function alignment. I've added this to our datalayouts to match LLVM (which is something we check) and strip the fnptr alignment for older LLVM versions.
 * The fmul/fadd reductions now always respect the accumulator (including for unordered reductions), so we should pass the identity instead of undef.

Open issues:

 * https://reviews.llvm.org/D62106 causes linker errors with ld.bdf due to https://sourceware.org/bugzilla/show_bug.cgi?id=24784. To avoid this I've enabled `RelaxELFRelocations`, which results in a GOTPCRELX relocation for `__tls_get_addr` and avoids the issue. However, this is likely not acceptable because relax relocations are not supported by older linker versions. We may need an LLVM option to keep using PLT for `__tls_get_addr` despite `RtLibUseGOT`.

The corresponding llvm-project PR is rust-lang/llvm-project#19.

r? @ghost

alexcrichton and others added some commits May 22, 2019

Fix compilation of sanitizers in Rust containers
It's not entertirely clear why this is necessary but this is carrying
over an old `compiler-rt` patch to ensure that `compiler-rt` compiles in
our super ancient containers that we build sanitizers in. This ideally
isn't the worst thing to keep with us going forward, but we'll see!
Fix sanitizer build without O_CLOEXEC
Define it to 0 if it doesn't exist.
[SLP] Optimize getSpillCost(); NFCI
For a given set of live values, the spill cost will always be the
same for each call. Compute the cost once and multiply it by the
number of calls.

(I'm not sure this spill cost modeling makes sense if there are
multiple calls, as the spill cost will likely be shared across
calls in that case. But that's how it currently works.)

llvm-svn: 365552
[X86] -fno-plt: use GOT __tls_get_addr only if GOTPCRELX is enabled
Summary:
As of binutils 2.32, ld has a bogus TLS relaxation error when the GD/LD
code sequence using R_X86_64_GOTPCREL (instead of R_X86_64_GOTPCRELX) is
attempted to be relaxed to IE/LE (binutils PR24784). gold and lld are good.

In gcc/config/i386/i386.md, there is a configure-time check of as/ld
support and the GOT relaxation will not be used if as/ld doesn't support
it:

    if (flag_plt || !HAVE_AS_IX86_TLS_GET_ADDR_GOT)
      return "call\t%P2";
    return "call\t{*%p2@GOT(%1)|[DWORD PTR %p2@GOT[%1]]}";

In clang, -DENABLE_X86_RELAX_RELOCATIONS=OFF is the default. The ld.bfd
bogus error can be reproduced with:

    thread_local int a;
    int main() { return a; }

clang -fno-plt -fpic a.cc -fuse-ld=bfd

GOTPCRELX gained relative good support in 2016, which is considered
relatively new.  It is even difficult to conditionally default to
-DENABLE_X86_RELAX_RELOCATIONS=ON due to cross compilation reasons. So
work around the ld.bfd bug by only using GOT when GOTPCRELX is enabled.

Reviewers: dalias, hjl.tools, nikic, rnk

Reviewed By: nikic

Subscribers: llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D64304

llvm-svn: 365752

@nikic nikic force-pushed the nikic:llvm-9 branch from f714d96 to 56b8425 Jul 11, 2019

@nikic nikic changed the title [WIP] Update to LLVM 9 Update to LLVM 9 Jul 11, 2019

@nikic nikic referenced this pull request Jul 11, 2019

Merged

Update to LLVM 9 trunk #62592

@alexcrichton alexcrichton merged commit 56b8425 into rust-lang:rustc/9.0-2019-07-08 Jul 11, 2019

bors added a commit to rust-lang/rust that referenced this pull request Jul 11, 2019

Auto merge of #62592 - nikic:actually-update-llvm, r=alexcrichton
Update to LLVM 9 trunk

Updates LLVM submodule to rust-lang/llvm-project#19 and:

 * Changes the LLVM Rust bindings to account for the new SubtargetSubTypeKV.
 * Adjusts a codegen test for the new form of the byval attribute that takes a type.
 * Disables LLDB on builders.

r? @alexcrichton

bors added a commit to rust-lang/rust that referenced this pull request Jul 12, 2019

Auto merge of #62592 - nikic:actually-update-llvm, r=alexcrichton
Update to LLVM 9 trunk

Updates LLVM submodule to rust-lang/llvm-project#19 and:

 * Changes the LLVM Rust bindings to account for the new SubtargetSubTypeKV.
 * Adjusts a codegen test for the new form of the byval attribute that takes a type.
 * Disables LLDB on builders.

r? @alexcrichton

bors added a commit to rust-lang/rust that referenced this pull request Jul 12, 2019

Auto merge of #62592 - nikic:actually-update-llvm, r=<try>
Update to LLVM 9 trunk

Updates LLVM submodule to rust-lang/llvm-project#19 and:

 * Changes the LLVM Rust bindings to account for the new SubtargetSubTypeKV.
 * Adjusts a codegen test for the new form of the byval attribute that takes a type.
 * Disables LLDB on builders.

r? @alexcrichton

bors added a commit to rust-lang/rust that referenced this pull request Jul 13, 2019

Auto merge of #62592 - nikic:actually-update-llvm, r=<try>
Update to LLVM 9 trunk

Updates LLVM submodule to rust-lang/llvm-project#19 and:

 * Changes the LLVM Rust bindings to account for the new SubtargetSubTypeKV.
 * Adjusts a codegen test for the new form of the byval attribute that takes a type.
 * Disables LLDB on builders.

r? @alexcrichton

bors added a commit to rust-lang/rust that referenced this pull request Jul 13, 2019

Auto merge of #62592 - nikic:actually-update-llvm, r=<try>
Update to LLVM 9 trunk

Updates LLVM submodule to rust-lang/llvm-project#19 and:

 * Changes the LLVM Rust bindings to account for the new SubtargetSubTypeKV.
 * Adjusts a codegen test for the new form of the byval attribute that takes a type.
 * Disables LLDB on builders.

r? @alexcrichton
@glandium

This comment has been minimized.

Copy link

commented Jul 18, 2019

Please avoid ending up releasing a version of rust that uses a non-released version of LLVM if you can.

@cuviper

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

@glandium For better or worse, it wouldn't be the first time for rustc to release with a snapshot LLVM. Since rust-lang/rust#62592 already landed, we're on track for Rust 1.38 to ship with this pre-release.

$ rustc +nightly -Vv
rustc 1.38.0-nightly (bc2e84ca0 2019-07-17)
binary: rustc
commit-hash: bc2e84ca0939b73fcf1768209044432f6a15c2e5
commit-date: 2019-07-17
host: x86_64-unknown-linux-gnu
release: 1.38.0-nightly
LLVM version: 9.0
@glandium

This comment has been minimized.

Copy link

commented Jul 18, 2019

sigh

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

@glandium FWIW you don't really mention a "why" in your comment and simply commenting "sigh" is pretty unlikely to actually get anything to happen (and comes of as pretty passive-aggressive?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.