Skip to content
Closed
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
30 changes: 20 additions & 10 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,27 +449,37 @@ impl Cargo {
let cc = ccacheify(&builder.cc(target));
self.command.env(format!("CC_{triple_underscored}"), &cc);

// Compiling C deps, like jemalloc and llvm-wrapper, should be with
// the same LTO mode as the Rust code they are linked into.
// Compiling C deps, like jemalloc and llvm-wrapper, should use the same LTO mode as
// the Rust code they are linked into. We don't pass the checks cc-rs uses to

@Zoxc Zoxc Aug 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be with the same LTO mode as the Rust code they are linked into
Just a note that this doesn't seem very true.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW, the discussion happens in #t-compiler > Analysing broken jemalloc rlib

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just a note that this doesn't seem very true.

I just reformatted that comment, I don't know if it is correct.
What would be more correct?

// auto-enable that so we have to do it ourselves.
//
// Std's C deps, e.g. compiler_builtins, ship inside rlibs that
// end users consume directly. Building with `-flto` may break
// non-LLVM linkers or mismatch on bitcode versions. Therefore we
// must not build std in LTO mode here.
// However, some of these files get distributed via rustup, and the user may then use
// non-LLVM linkers. If we turn on regular LTO (which stores LLVM bitcode and nothing
// else), only LLVM linkers can deal with those files. Therefore we have to use "fat
// LTO" and include both machine code and LLVM bitcode.
//
// We don't do this for `std` as this may make std bigger and it is generally a
// non-trivial configuration we do not want to risk. (`std` does have C deps, e.g.
// compiler_builtins, so this is relevant.)
let lto_cflag = if matches!(self.mode, Mode::Rustc | Mode::ToolRustcPrivate)

@RalfJung RalfJung Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FWIW, this matches! here doesn't really make sense, given that we distribute rustc-dev so those files also have to be in a shape where users need to be able to link them. All it does is restrict breakage to folks that e.g. build Miri, and making it harder for us to notice breakage...

View changes since the review

&& is_lto_stage(&self.compiler)
&& builder.cc_tool(target).is_like_clang()
Comment thread
weihanglo marked this conversation as resolved.
{
match builder.config.rust_lto {
RustcLto::Thin => Some("-flto=thin"),
RustcLto::Fat => Some("-flto=full"),
let lto_cflag = match builder.config.rust_lto {
RustcLto::Thin => Some("-flto=thin -ffat-lto-objects"),
RustcLto::Fat => Some("-flto=full -ffat-lto-objects"),
RustcLto::ThinLocal | RustcLto::Off => None,
};
// Make sure the linker actually uses the fat LTO part.
if lto_cflag.is_some() {
self.rustflags.arg("-Clink-args=-Wl,--fat-lto-objects");

@RalfJung RalfJung Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

IIUC, this flag is needed to make the linker even recognize the bitcode in fat LTO objects. It is strange that this is not the default (unlike LTO for "thin" objects which apparently is the default). Does this mean we're also not LTO'ing jemalloc for the distributed Miri, Clippy etc any more?

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll do a Clippy benchmark after the current one finishes to check.

@mati865 mati865 Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this is LLD specific flag and you can let Clang handle it itself.
You can check that using clang hello_world.c -flto -ffat-lto-objects -###. With -fuse-ld=lld it will automatically add --fat-lto-objects to the linker invocation.

Linkers other than LLD don't need this flag because LLVMgold.so plugin should handle it on its own.

@RalfJung RalfJung Aug 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The flag was needed, we lost LTO otherwise which was visible in perf runs. Don't ask me why, I am throwing spaghetti at the wall here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant it also works when you pass -Clink-args=-ffat-lto-objects rather than -Clink-args=-Wl,--fat-lto-objects because Clang automatically adds --fat-lto-objects to the linker invocation if using LLD.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah okay, thanks. We didn't try that.

}
lto_cflag
} else {
None
};

// Extend `CXXFLAGS_$TARGET` with our extra flags.
// Extend `CFLAGS_$TARGET` with our extra flags.
let env = format!("CFLAGS_{triple_underscored}");
let mut cflags =
builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C).join(" ");
Expand Down
Loading