-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Optimize librustc_driver.so
with BOLT
#116352
Changes from all commits
240a7dd
589e38a
dd7c5a0
9a0e90f
482a820
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -906,6 +906,11 @@ impl Step for Rustc { | |
cargo.arg("-p").arg(krate); | ||
} | ||
|
||
if builder.build.config.enable_bolt_settings && compiler.stage == 1 { | ||
// Relocations are required for BOLT to work. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would've expected this if to gate on the usage of BOLT somehow -- do we have a config flag for that? I think just a generic config flag is fine ("enable-bolt-settings") is ok, we don't need it to actually do optimizations. Maybe even "opt-dist" is the right name for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what I have discussed in the PR description and on Zulip. @onur-ozkan has expressed a desire to avoid a BOLT-specific flag, although it seems like the most straightforward solution to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have problem with doing this with using flags/config.toml. My point was more about its behaviour.
Like here, "which would set the flag only for the cases where it's needed" this seems very limited experience to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the interaction of the flag seems very specific. However, I feel like that's also true in general of several other similar flags. For example, the PGO generate/use flags are also only performed in stage 2, and basically I don't think that they are very useful outside of The fact that we pass a specific linker flag is tied to BOLT - that won't really change. The fact that we only pass it to bootstrap and |
||
cargo.env("RUSTC_BOLT_LINK_FLAGS", "1"); | ||
} | ||
|
||
let _guard = builder.msg_sysroot_tool( | ||
Kind::Build, | ||
compiler.stage, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added after the debug printing, so the extra flags won't show up when the command is printed. Was that deliberate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely not, good catch, thanks! #123189