-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Automatically switch to lto-fat when flag RUSTFLAGS="- Zautodiff=Enable" is set #146229
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
Conversation
This comment has been minimized.
This comment has been minimized.
1d24e80
to
bbea16e
Compare
This comment has been minimized.
This comment has been minimized.
bbea16e
to
7389d18
Compare
r? @ZuseZ4 hi, please take a look at this PR, |
|
||
/// Calculates the flavor of LTO to use for this compilation. | ||
pub fn lto(&self) -> config::Lto { | ||
// if autodiff is enabled, use fat lto |
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.
Can you change this to explain why we do it? Something along the lines of
// Autodiff currently requires fat-lto to have access to the llvm-ir of all (indirectly) used functions and types. fat-lto is the easiest solution to this requirement, but quite expensive. FIXME(autodiff): Make autodiff also work with embed-bc instead of fat-lto.
When you change it, please update your last commit and force push, don't make a second commit just for doc changes.
Otherwise lgtm. Sorry for taking so long, I was traveling and wanted to test it locally.
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.
Also, while already at it, can you please remove the now unneeded (and already unused) error related to it? There's also a diagnostics in a .ftl file.
compiler/rustc_codegen_ssa/src/errors.rs
42:pub struct AutodiffWithoutLto;
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.
Okay, I'll take care of this today.
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.
I have remove code:
#[derive(Diagnostic)]
#[diag(codegen_ssa_autodiff_without_lto)]
pub struct AutodiffWithoutLto;
I found, the AutodiffWithoutLto is declared but has never been used?
7389d18
to
4b06752
Compare
Some changes occurred in compiler/rustc_codegen_ssa |
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
4b06752
to
6e74905
Compare
Thanks! Yes the error logic was removed during a previous refactoring, which Iirc made a little it harder to check for it. Since the plan was to enable lto automatically I didn't want to block a larger refactor based on it. @bors r+ rollup |
Automatically switch to lto-fat when flag RUSTFLAGS="- Zautodiff=Enable" is set …t" is automatically set. closes: [rust-lang#142796](rust-lang#142796)
Rollup of 8 pull requests Successful merges: - #146229 (Automatically switch to lto-fat when flag RUSTFLAGS="- Zautodiff=Enable" is set) - #146615 (rustc_codegen_llvm: Feature Conversion Tidying) - #146638 (`rustc_next_trait_solver`: canonical out of `EvalCtxt`) - #146663 (Allow windows resource compiler to be overridden) - #146691 (std: Fix WASI implementation of `remove_dir_all`) - #146709 (stdarch subtree update) - #146731 (test: Use SVG for terminal url test) - #146738 (Fix tidy spellchecking on Windows) r? `@ghost` `@rustbot` modify labels: rollup
Automatically switch to lto-fat when flag RUSTFLAGS="- Zautodiff=Enable" is set …t" is automatically set. closes: [rust-lang#142796](rust-lang#142796)
Rollup of 10 pull requests Successful merges: - #146229 (Automatically switch to lto-fat when flag RUSTFLAGS="- Zautodiff=Enable" is set) - #146484 (rustdoc-search: JavaScript optimization based on Firefox Profiler output) - #146541 (std: simplify host lookup) - #146615 (rustc_codegen_llvm: Feature Conversion Tidying) - #146638 (`rustc_next_trait_solver`: canonical out of `EvalCtxt`) - #146663 (Allow windows resource compiler to be overridden) - #146691 (std: Fix WASI implementation of `remove_dir_all`) - #146709 (stdarch subtree update) - #146738 (Fix tidy spellchecking on Windows) - #146740 (miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 10 pull requests Successful merges: - rust-lang/rust#146229 (Automatically switch to lto-fat when flag RUSTFLAGS="- Zautodiff=Enable" is set) - rust-lang/rust#146484 (rustdoc-search: JavaScript optimization based on Firefox Profiler output) - rust-lang/rust#146541 (std: simplify host lookup) - rust-lang/rust#146615 (rustc_codegen_llvm: Feature Conversion Tidying) - rust-lang/rust#146638 (`rustc_next_trait_solver`: canonical out of `EvalCtxt`) - rust-lang/rust#146663 (Allow windows resource compiler to be overridden) - rust-lang/rust#146691 (std: Fix WASI implementation of `remove_dir_all`) - rust-lang/rust#146709 (stdarch subtree update) - rust-lang/rust#146738 (Fix tidy spellchecking on Windows) - rust-lang/rust#146740 (miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup
…t" is automatically set.
closes: #142796