Skip to content

Commit 6b1e353

Browse files
committed
Fix linker-plugin-lto only doing thin lto
When rust provides LLVM bitcode files to lld and the bitcode contains function summaries as used for thin lto, lld defaults to using thin lto. This prevents some optimizations that are only applied for fat lto. I ran into this with the amdgpu backend and was able to create a test there. Unfortunately, I wasn’t able to recreate the same test on x86, the difference between thin and fat lto seems less pronounced there. The important part of the change is setting the `ThinLTO=0` module flag. The rest of the changes are fixing the `<function>.kd` symbol getting exported for amdhsa with linker-plugin-lto.
1 parent c03c38d commit 6b1e353

File tree

6 files changed

+95
-4
lines changed

6 files changed

+95
-4
lines changed

compiler/rustc_codegen_llvm/src/context.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ use rustc_middle::ty::layout::{
2121
};
2222
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
2323
use rustc_middle::{bug, span_bug};
24-
use rustc_session::Session;
2524
use rustc_session::config::{
2625
BranchProtection, CFGuard, CFProtection, CrateType, DebugInfo, FunctionReturn, PAuthKey, PacRet,
2726
};
27+
use rustc_session::{Session, config};
2828
use rustc_span::source_map::Spanned;
2929
use rustc_span::{DUMMY_SP, Span};
3030
use rustc_target::spec::{HasTargetSpec, RelocModel, SmallDataThresholdSupport, Target, TlsModel};
@@ -290,6 +290,11 @@ pub(crate) unsafe fn create_module<'ll>(
290290
);
291291
}
292292

293+
// Disable ThinLTO if fat lto is requested. Otherwise lld defaults to thin lto.
294+
if sess.lto() == config::Lto::Fat {
295+
llvm::add_module_flag_u32(llmod, llvm::ModuleFlagMergeBehavior::Override, "ThinLTO", 0);
296+
}
297+
293298
// Add "kcfi" module flag if KCFI is enabled. (See https://reviews.llvm.org/D119296.)
294299
if sess.is_sanitizer_kcfi_enabled() {
295300
llvm::add_module_flag_u32(llmod, llvm::ModuleFlagMergeBehavior::Override, "kcfi", 1);

compiler/rustc_codegen_ssa/src/back/link.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2549,8 +2549,14 @@ fn add_order_independent_options(
25492549
// as it appears to be unused. This can then cause the PGO profile file to lose
25502550
// some functions. If we are generating a profile we shouldn't strip those metadata
25512551
// sections to ensure we have all the data for PGO.
2552-
let keep_metadata =
2553-
crate_type == CrateType::Dylib || sess.opts.cg.profile_generate.enabled();
2552+
let dylib_pgo = crate_type == CrateType::Dylib || sess.opts.cg.profile_generate.enabled();
2553+
// When compiling for amdhsa, every kernel function generates a <function name>.kd symbol.
2554+
// This symbol gets removed again when using linker-plugin-lto. Disable gc_sections to keep
2555+
// the symbol.
2556+
let amdhsa_linker_lto =
2557+
sess.target.os == "amdhsa" && sess.opts.cg.linker_plugin_lto.enabled();
2558+
2559+
let keep_metadata = dylib_pgo || amdhsa_linker_lto;
25542560
if crate_type != CrateType::Executable || !sess.opts.unstable_opts.export_executable_symbols
25552561
{
25562562
cmd.gc_sections(keep_metadata);

compiler/rustc_codegen_ssa/src/back/linker.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,20 @@ impl<'a> Linker for GccLinker<'a> {
839839
} else {
840840
let mut arg = OsString::from("--version-script=");
841841
arg.push(path);
842-
self.link_arg(arg).link_arg("--no-undefined-version");
842+
self.link_arg(arg);
843+
844+
if self.sess.target.os == "amdhsa" && self.sess.opts.cg.linker_plugin_lto.enabled() {
845+
// If we are compiling for amdhsa, kernel functions generate a symbol
846+
// <function name>.kd in the LLVM backend, which needs to be visible in the output.
847+
// But if we give bitcode to the linker, the symbol is not yet defined and the
848+
// linker complains about an undefined symbol in the version script.
849+
// At the same time, if we do not specify the <function name>.kd symbol in the
850+
// version script, it will not be exported.
851+
// Workaround by allowing the version script do contain undefined symbols.
852+
self.link_arg("--undefined-version");
853+
} else {
854+
self.link_arg("--no-undefined-version");
855+
}
843856
}
844857
}
845858

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#![feature(no_core, lang_items)]
2+
#![no_core]
3+
#![crate_type = "staticlib"]
4+
5+
#[lang = "sized"]
6+
trait Sized {}
7+
8+
#[no_mangle]
9+
extern "C" fn foo() {}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![feature(no_core, abi_gpu_kernel, lang_items)]
2+
#![no_core]
3+
#![crate_type = "cdylib"]
4+
5+
#[lang = "sized"]
6+
trait Sized {}
7+
8+
extern "C" {
9+
fn foo();
10+
}
11+
12+
#[no_mangle]
13+
extern "gpu-kernel" fn bar() {
14+
unsafe {
15+
foo();
16+
}
17+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Check that lto with -C linker-plugin-lto actually works and can inline functions.
2+
// A static library is compiled, defining a function. Then a dylib is compiled,
3+
// linking to the static library and calling the function from the library.
4+
// The function from the library should end up inlined and disappear from the output.
5+
6+
//@ needs-llvm-components: amdgpu
7+
//@ needs-rust-lld
8+
9+
#![feature(path_file_prefix)]
10+
11+
use run_make_support::{dynamic_lib_name, llvm_readobj, rustc, static_lib_name};
12+
13+
fn main() {
14+
rustc()
15+
.input("lib.rs")
16+
.output(static_lib_name("lib"))
17+
.args(&["-Clinker-plugin-lto", "--target=amdgcn-amd-amdhsa", "-Ctarget-cpu=gfx900"])
18+
.run();
19+
rustc()
20+
.input("main.rs")
21+
.output("main.elf")
22+
.opt_level("3")
23+
.args(&[
24+
"-Clinker-plugin-lto",
25+
"--target=amdgcn-amd-amdhsa",
26+
"-Ctarget-cpu=gfx900",
27+
&format!("-Clink-arg={}", static_lib_name("lib")),
28+
])
29+
.run();
30+
31+
llvm_readobj()
32+
.input("main.elf")
33+
.symbols()
34+
.run()
35+
// The linked function should disappear.
36+
.assert_stdout_not_contains("foo")
37+
// The function from main.rs should still be there
38+
.assert_stdout_contains("bar")
39+
// and the kernel descriptor .kd symbol as well.
40+
.assert_stdout_contains("bar.kd");
41+
}

0 commit comments

Comments
 (0)