Skip to content

Conversation

@madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Nov 24, 2025

This allows LTO to work when compiling jemalloc (which is currently broken due to rust-lang/cc-rs#1613), which should yield a small performance boost, and makes Miri's behaviour here match Clippy
and Rustdoc.

Follow-up to #148925 / #146627 after discussion in #148925 (review).

r? RalfJung

This allows LTO to work when compiling jemalloc, which should yield a
small performance boost, and makes miri's behaviour here match clippy
and rustdoc.
@madsmtm madsmtm added O-linux Operating system: Linux O-macos Operating system: macOS A-allocators Area: Custom and system allocators A-miri Area: The miri tool A-linkers Area: linkers... you gotta love linkers labels Nov 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 24, 2025

The Miri subtree was changed

cc @rust-lang/miri

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Nov 24, 2025
@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 24, 2025

Does this need a perf run? I don't think Miri is tested in rustc-perf?

@saethlin
Copy link
Member

saethlin commented Nov 24, 2025

Miri has its own benchmark suite based on hyperfine that you can run with ./miri bench but that's designed for use out of rust-lang/miri so I don't know if it works in-tree.

I'd be happy merging this without running any benchmarks.

add_features: |builder, target, features| {
if builder.config.jemalloc(target) {
features.push("jemalloc".to_string());
}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean the feature will be set for ./x build miri but not for ./x test miri?

extern crate rustc_span;

/// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
/// and https://github.com/rust-lang/rust/pull/146627 for why we need this `use` statement.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// and https://github.com/rust-lang/rust/pull/146627 for why we need this `use` statement.
/// and https://github.com/rust-lang/rust/pull/146627 for why we need this.

///
/// FIXME(madsmtm): This is loaded from the sysroot that was built with the other `rustc` crates
/// above, instead of via Cargo as you'd normally do. This is currently needed for LTO due to
/// https://github.com/rust-lang/cc-rs/issues/1613.
Copy link
Member

Choose a reason for hiding this comment

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

This also saves the time for building jemalloc again so that seems nice 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-allocators Area: Custom and system allocators A-linkers Area: linkers... you gotta love linkers A-miri Area: The miri tool O-linux Operating system: Linux O-macos Operating system: macOS S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants