Upgrade to LLVM 13

[nikic]

Work in progress update to LLVM 13. Main changes:

• InlineAsm diagnostics reported using SrcMgr diagnostic kind are now handled. Previously these used a separate diag handler.
• Codegen tests are updated for additional attributes.
• Some data layouts have changed.
• Switch #[used] attribute from llvm.used to llvm.compiler.used to avoid SHF_GNU_RETAIN flag introduced in https://reviews.llvm.org/D97448, which appears to trigger a bug in older versions of gold.
• Set LLVM_INCLUDE_TESTS=OFF to avoid Python 3.6 requirement.

Upstream issues:

• https://bugs.llvm.org/show_bug.cgi?id=51210 (InlineAsm diagnostic reporting for module asm) Fixed by llvm/llvm-project@1558bb8.
• https://bugs.llvm.org/show_bug.cgi?id=51476 (Miscompile on AArch64 due to incorrect comparison elimination) Fixed by llvm/llvm-project@81b1065.
• https://bugs.llvm.org/show_bug.cgi?id=51207 (Can't set custom section flags anymore). Problematic change reverted in our fork, https://reviews.llvm.org/D107216 posted for upstream revert.
• https://bugs.llvm.org/show_bug.cgi?id=51211 (Regression in codegen for && operator chains (and ||, possibly) generates unoptimizable LLVM IR #83623). This is an optimization regression that we may likely have to eat for this release. The fix for && operator chains (and ||, possibly) generates unoptimizable LLVM IR #83623 was based on an incorrect premise, and this needs to be properly addressed in the MergeICmps pass.

The compile-time impact is mixed, but quite positive as LLVM upgrades go.

The LLVM 13 final release is scheduled for Sep 21st. The current nightly is scheduled for stable release on Oct 21st.

r? @ghost

[nikic]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit ce064a239e234c6b926107fd9382b9cda7589e83 with merge 450db03509584046c77604194f41a448ac97e1c5...

[bors]

☀️ Try build successful - checks-actions
Build commit: 450db03509584046c77604194f41a448ac97e1c5 (450db03509584046c77604194f41a448ac97e1c5)

[rust-timer]

Queued 450db03509584046c77604194f41a448ac97e1c5 with parent a28109a, future comparison URL.

[rust-timer]

Finished benchmarking try commit (450db03509584046c77604194f41a448ac97e1c5): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

• Very large regression in instruction counts (up to 31.7% on incr-unchanged builds of helloworld-opt)
• Very large improvement in instruction counts (up to -10.5% on incr-unchanged builds of helloworld-check)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[nikic]

It looks like the large regressions at the start are all slowdowns in run_linker, so this might be related to the missing llvmbc section flag.

[hkratz]

It looks like the large regressions at the start are all slowdowns in run_linker, so this might be related to the missing llvmbc section flag.

Seems like it. The resulting binaries do get bloated with bitcode.

The resulting stripped binary of compiling the helloworld perf test...

with Rust nightly 2021-07-28:

rustc -C opt-level=3 src/main.rs;strip main;ls -l main
-rwxr-xr-x 1 hans hans 272656 Jul 29 09:54 main

with LLVM 13:

~/dev/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustc -C opt-level=3 src/main.rs;strip main;ls -l main
-rwxr-xr-x 1 hans hans 6384232 Jul 29 10:07 main

Just backing out the hack and reverting the LLVM commit to allow the SHF_EXCLUDE flag on .llvmbc again works and makes it better:

~/dev/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustc -C opt-level=3 src/main.rs;strip main;ls -l main
-rwxr-xr-x 1 hans hans 309520 Jul 29 09:56 main

[nikic]

Weird issue encountered via the pgo-branch-weights test:

fn main() {
    println!("{:?}", std::env::args());
}

Running rustc test.rs && ./test works fine. Running rustc -Clink-args=-fuse-ld=gold test.rs && ./test gives empty arguments.

[nikic]

Just backing out the hack and reverting the LLVM commit to allow the SHF_EXCLUDE flag on .llvmbc again works and makes it better:

~/dev/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustc -C opt-level=3 src/main.rs;strip main;ls -l main
-rwxr-xr-x 1 hans hans 309520 Jul 29 09:56 main

Unfortunately, it seems like this doesn't actually work: Due to https://reviews.llvm.org/D100944 this no longer modifies the section flags on .llvmbc and will instead emit first an empty .llvmbc section with EXCLUDE, and then the actual .llvmbc section with ALLOC. LLVM is then no longer able to find the bitcode.

[nikic]

The good news is that reverting (just) https://reviews.llvm.org/D100944 does fix the llvmbc issue.

Running rustc test.rs && ./test works fine. Running rustc -Clink-args=-fuse-ld=gold test.rs && ./test gives empty arguments.

It looks like something is going wrong with .init_array:

objdump -t test_ld | grep init_array
0000000000048250 l    d  .init_array	0000000000000000              .init_array
0000000000048258 l     O .init_array	0000000000000000              __frame_dummy_init_array_entry
0000000000048250 l     O .init_array	0000000000000008              _ZN3std3sys4unix4args3imp15ARGV_INIT_ARRAY17hb13aa0f306b867eeE
0000000000048260 l       .init_array	0000000000000000              __init_array_end
0000000000048250 l       .init_array	0000000000000000              __init_array_start

objdump -t test_gold | grep init_array
000000000004a218 l     O .init_array	0000000000000000              __frame_dummy_init_array_entry
000000000004c728 l     O .init_array	0000000000000008              _ZN3std3sys4unix4args3imp15ARGV_INIT_ARRAY17hb13aa0f306b867eeE
000000000004a218 l       .init_array	0000000000000000              .hidden __init_array_start
000000000004a220 l       .init_array	0000000000000000              .hidden __init_array_end

Notably, with ld we have ARGV_INIT_ARRAY sitting between __init_array_start and __init_array_end. With gold it's located after __init_array_end.

[nikic]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 5f2b5c7078195e844f3b07fcad2a13172882c68f with merge 0983094463497eec22d550dad25576a894687002...

[bors]

☀️ Try build successful - checks-actions
Build commit: 0983094463497eec22d550dad25576a894687002 (0983094463497eec22d550dad25576a894687002)

[rust-timer]

Queued 0983094463497eec22d550dad25576a894687002 with parent ef9549b, future comparison URL.

[rust-timer]

Finished benchmarking try commit (0983094463497eec22d550dad25576a894687002): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

• Very large improvement in instruction counts (up to -10.4% on incr-unchanged builds of helloworld-check)
• Large regression in instruction counts (up to 9.6% on full builds of match-stress-enum-check)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[nikic]

Still rather stumped on the gold issue and don't really know how to debug it.

The best lead I have right now is that there are now some adjacent anon globals in the .data.rel.ro section, which were not present previously. Extracting the bitcode from the stdlib rlib, the relevant difference seems to be that a number of panic message constants are now hidden unnamed_addr constant, while they were previously private unnamed_addr constant. This means that these symbols now have external linkage, which they shouldn't have.

[tmiasko]

Running rustc test.rs && ./test works fine. Running rustc -Clink-args=-fuse-ld=gold test.rs && ./test gives empty arguments.

#[used] attribute now results in SHF_GNU_RETAIN / R flag being set https://reviews.llvm.org/D97448, which seems to be causing the issue when linking with gold < binutils 2.36.

[nikic]

@tmiasko Ooh, nice find!

As suggested in the differential, we might want to switch to llvm.compiler.used then, as all our docs on #[used] make it abundantly clear that the attribute is only supposed to ensure that the symbol makes it into the object file, but the linker is still allowed to drop it.

[leonardo-m]

This has reduced by about 20% the number of array bound panics (and other similar panic) in my binaries (despite the final binary size is few percent bigger), this is a significant (good) change, I don't yet know what caused it but I'll try to understand it.

[adrian17]

despite the final binary size is few percent bigger

I also observe 1-3% size increases in final binaries (including wasm); not sure if that is to be expected, or warrants an issue.

[leonardo-m]

not sure if that is to be expected, or warrants an issue.

In my opinion this kind of slow little change is better tracked with specialized efforts (like the one for the compiler performance).

[briansmith]

Upgrading to LLVM changes the profiling data format incompatibly compared to LLVM 12, breaking my CI/CD code coverage jobs. Thus nightly users doing source-based code coverage measurement LLVM 13's profiling tools. However, LLVM 13 hasn't been released yet, so it looks like people need to use https://github.com/llvm/llvm-project/releases/tag/llvmorg-13.0.0-rc1 for now.

[Mark-Simulacrum]

We ship the necessary tools as part of llvm-tools-preview as far as I know, so I believe that should always work.

[briansmith]

We ship the necessary tools as part of llvm-tools-preview as far as I know, so I believe that should always work.

Thanks for that pointer!

If you are building non-Rust code then you'll also need LLVM-13 compatible tools to use those new tools, e.g. clang-13.