Skip to content

Fix linker-plugin-lto only doing thin lto #136840

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ pub(crate) unsafe fn codegen(
"LLVM_module_codegen_make_bitcode",
&*module.name,
);
ThinBuffer::new(llmod, config.emit_thin_lto, false)
ThinBuffer::new(llmod, cgcx.lto != Lto::Fat && config.emit_thin_lto, false)
};
let data = thin.data();
let _timer = cgcx
Expand Down
30 changes: 30 additions & 0 deletions src/tools/run-make-support/src/external_deps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ pub fn llvm_pdbutil() -> LlvmPdbutil {
LlvmPdbutil::new()
}

/// Construct a new `llvm-as` invocation. This assumes that `llvm-as` is available
/// at `$LLVM_BIN_DIR/llvm-as`.
pub fn llvm_as() -> LlvmAs {
LlvmAs::new()
}

/// Construct a new `llvm-dis` invocation. This assumes that `llvm-dis` is available
/// at `$LLVM_BIN_DIR/llvm-dis`.
pub fn llvm_dis() -> LlvmDis {
Expand Down Expand Up @@ -135,6 +141,13 @@ pub struct LlvmPdbutil {
cmd: Command,
}

/// A `llvm-as` invocation builder.
#[derive(Debug)]
#[must_use]
pub struct LlvmAs {
cmd: Command,
}

/// A `llvm-dis` invocation builder.
#[derive(Debug)]
#[must_use]
Expand All @@ -158,6 +171,7 @@ crate::macros::impl_common_helpers!(LlvmNm);
crate::macros::impl_common_helpers!(LlvmBcanalyzer);
crate::macros::impl_common_helpers!(LlvmDwarfdump);
crate::macros::impl_common_helpers!(LlvmPdbutil);
crate::macros::impl_common_helpers!(LlvmAs);
crate::macros::impl_common_helpers!(LlvmDis);
crate::macros::impl_common_helpers!(LlvmObjcopy);

Expand Down Expand Up @@ -441,6 +455,22 @@ impl LlvmObjcopy {
}
}

impl LlvmAs {
/// Construct a new `llvm-as` invocation. This assumes that `llvm-as` is available
/// at `$LLVM_BIN_DIR/llvm-as`.
pub fn new() -> Self {
let llvm_as = llvm_bin_dir().join("llvm-as");
let cmd = Command::new(llvm_as);
Self { cmd }
}

/// Provide an input file.
pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
self.cmd.arg(path.as_ref());
self
}
}

impl LlvmDis {
/// Construct a new `llvm-dis` invocation. This assumes that `llvm-dis` is available
/// at `$LLVM_BIN_DIR/llvm-dis`.
Expand Down
6 changes: 6 additions & 0 deletions src/tools/run-make-support/src/external_deps/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ impl Rustc {
self
}

/// This flag enables LTO in the specified form.
pub fn lto(&mut self, option: &str) -> &mut Self {
self.cmd.arg(format!("-Clto={option}"));
self
}

/// This flag defers LTO optimizations to the linker.
pub fn linker_plugin_lto(&mut self, option: &str) -> &mut Self {
self.cmd.arg(format!("-Clinker-plugin-lto={option}"));
Expand Down
2 changes: 1 addition & 1 deletion src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub use cargo::cargo;
pub use clang::{clang, Clang};
pub use htmldocck::htmldocck;
pub use llvm::{
llvm_ar, llvm_bcanalyzer, llvm_dis, llvm_dwarfdump, llvm_filecheck, llvm_nm, llvm_objcopy,
llvm_ar, llvm_bcanalyzer, llvm_as, llvm_dis, llvm_dwarfdump, llvm_filecheck, llvm_nm, llvm_objcopy,
llvm_objdump, llvm_profdata, llvm_readobj, LlvmAr, LlvmBcanalyzer, LlvmDis, LlvmDwarfdump,
LlvmFilecheck, LlvmNm, LlvmObjcopy, LlvmObjdump, LlvmProfdata, LlvmReadobj,
};
Expand Down
27 changes: 20 additions & 7 deletions tests/run-make/cross-lang-lto-clang/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,24 @@ static C_NEVER_INLINED_PATTERN: &'static str = "bl.*<c_never_inlined>";
static C_NEVER_INLINED_PATTERN: &'static str = "call.*c_never_inlined";

fn main() {
test_lto(false);
test_lto(true);
}

fn test_lto(fat_lto: bool) {
let lto = if fat_lto { "fat" } else { "thin" };
let clang_lto = if fat_lto { "full" } else { "thin" };

rustc()
.lto(lto)
.linker_plugin_lto("on")
.output(static_lib_name("rustlib-xlto"))
.opt_level("2")
.codegen_units(1)
.input("rustlib.rs")
.run();
clang()
.lto("thin")
.lto(clang_lto)
.use_ld("lld")
.arg("-lrustlib-xlto")
.out_exe("cmain")
Expand All @@ -57,9 +66,10 @@ fn main() {
.input("cmain")
.run()
.assert_stdout_contains_regex(RUST_NEVER_INLINED_PATTERN);
clang().input("clib.c").lto("thin").arg("-c").out_exe("clib.o").arg("-O2").run();
clang().input("clib.c").lto(clang_lto).arg("-c").out_exe("clib.o").arg("-O2").run();
llvm_ar().obj_to_ar().output_input(static_lib_name("xyz"), "clib.o").run();
rustc()
.lto(lto)
.linker_plugin_lto("on")
.opt_level("2")
.linker(&env_var("CLANG"))
Expand All @@ -72,9 +82,12 @@ fn main() {
.input("rsmain")
.run()
.assert_stdout_not_contains_regex(C_ALWAYS_INLINED_PATTERN);
llvm_objdump()
.disassemble()
.input("rsmain")
.run()
.assert_stdout_contains_regex(C_NEVER_INLINED_PATTERN);

let dump = llvm_objdump().disassemble().input("rsmain").run();
if !fat_lto {
dump.assert_stdout_contains_regex(C_NEVER_INLINED_PATTERN);
} else {
// fat lto inlines this anyway
dump.assert_stdout_not_contains_regex(C_NEVER_INLINED_PATTERN);
}
}
8 changes: 8 additions & 0 deletions tests/run-make/fat-then-thin-lto/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![feature(no_core, lang_items)]
#![no_core]
Comment on lines +1 to +2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jieyouxu Do you think this test should use minicore?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? As in, run-make tests don't get native //@ add-core-stubs / minicore support by default (not implemented initially).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering your response and the state of this PR, I'll let someone else clean this up later if they want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I didn't add minicore support for run-make when I implemented minicore initially because it's not always clear what about minicore run-make tests may want.

#![crate_type = "rlib"]

#[lang = "sized"]
trait Sized {}

pub fn foo() {}
11 changes: 11 additions & 0 deletions tests/run-make/fat-then-thin-lto/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![allow(internal_features)]
#![feature(no_core, lang_items)]
#![no_core]
#![crate_type = "cdylib"]

extern crate lib;

#[unsafe(no_mangle)]
pub fn bar() {
lib::foo();
}
25 changes: 25 additions & 0 deletions tests/run-make/fat-then-thin-lto/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Compile a library with lto=fat, then compile a binary with lto=thin
// and check that lto is applied with the library.
// The goal is to mimic the standard library being build with lto=fat
// and allowing users to build with lto=thin.

//@ only-x86_64-unknown-linux-gnu

use run_make_support::{dynamic_lib_name, llvm_objdump, rustc};

fn main() {
rustc().input("lib.rs").opt_level("3").lto("fat").run();
rustc().input("main.rs").panic("abort").opt_level("3").lto("thin").run();

llvm_objdump()
.input(dynamic_lib_name("main"))
.arg("--disassemble-symbols=bar")
.run()
// The called function should be inlined.
// Check that we have a ret (to detect tail
// calls with a jmp) and no call.
.assert_stdout_contains("bar")
.assert_stdout_contains("ret")
.assert_stdout_not_contains("foo")
.assert_stdout_not_contains("call");
}
6 changes: 6 additions & 0 deletions tests/run-make/linker-plugin-lto-fat/ir.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @ir_callee() {
ret void
}
17 changes: 17 additions & 0 deletions tests/run-make/linker-plugin-lto-fat/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(no_core, lang_items)]
#![no_core]
#![crate_type = "cdylib"]

#[lang = "sized"]
trait Sized {}

extern "C" {
fn ir_callee();
}

#[no_mangle]
extern "C" fn rs_foo() {
unsafe {
ir_callee();
}
}
32 changes: 32 additions & 0 deletions tests/run-make/linker-plugin-lto-fat/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Check that -C lto=fat with -C linker-plugin-lto actually works and can inline functions.
// A library is created from LLVM IR, defining a single function. Then a dylib is compiled,
// linking to the library and calling the function from the library.
// The function from the library should end up inlined and disappear from the output.

//@ only-x86_64-unknown-linux-gnu
//@ needs-rust-lld

use run_make_support::{dynamic_lib_name, llvm_as, llvm_objdump, rustc};

fn main() {
llvm_as().input("ir.ll").run();
rustc()
.input("main.rs")
.opt_level("3")
.lto("fat")
.linker_plugin_lto("on")
.link_arg("ir.bc")
.arg("-Zlinker-features=+lld")
.run();

llvm_objdump()
.input(dynamic_lib_name("main"))
.arg("--disassemble-symbols=rs_foo")
.run()
// The called function should be inlined.
// Check that we have a ret (to detect tail
// calls with a jmp) and no call.
.assert_stdout_contains("foo")
.assert_stdout_contains("ret")
.assert_stdout_not_contains("call");
}
Loading