Skip to content

refactor(mir-transform): Calculate optimization status inside run_passes_inner - #160057

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
clubby789:local-optimizations
Jul 28, 2026
Merged

refactor(mir-transform): Calculate optimization status inside run_passes_inner#160057
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
clubby789:local-optimizations

Conversation

@clubby789

Copy link
Copy Markdown
Contributor

The Optimizations enum represents whether #[optimize(none)] has been applied to a function. Currently, run_passes takes an Optimizations which is confusing (#160015 (comment)).
Calculate this in run_passes_inner instead (which also means #[optimize(none)] is applied more consistently.

r? @RalfJung

@rustbot

rustbot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 28, 2026
@rustbot

rustbot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

Comment thread compiler/rustc_mir_transform/src/pass_manager.rs Outdated
@RalfJung

Copy link
Copy Markdown
Member

This changes behavior, right? I am not sure how to figure out whether this has any bad fallout.

@clubby789
clubby789 force-pushed the local-optimizations branch 2 times, most recently from 053bad2 to c325709 Compare July 28, 2026 09:27
@rustbot

rustbot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in coverage instrumentation.

cc @Zalathar

@clubby789

Copy link
Copy Markdown
Contributor Author

This did introduce a regression in that InstrumentCoverage is currently is_required=false. That will be fixed by the other PR, but can be fixed here by making it required.

@RalfJung

Copy link
Copy Markdown
Member

Yeah the point of splitting it was to separate the functional change from the larger refactor. :)

Comment thread compiler/rustc_mir_transform/src/pass_manager.rs Outdated
@scottmcm

Copy link
Copy Markdown
Member

meta-pondering: Does optimize(none) really want to be different from opt-level=0?

Should maybe we stop checking mir-opt-level from the session in passes and instead check something passed-in only, with that passed-in thing merging the various knobs into one?

I would be sad if people started cargo-culting #[optimize(none)] onto things in debug because they heard that made it do less than opt-level=0...

@RalfJung RalfJung left a comment

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 think I found a few cases where this changes behavior -- previously we'd run SimplifyCfg, now we don't.

Those do seem like good changes, though I wonder how far we want to go with promising "no" optimizations...

View changes since this review

&add_call_guards::CriticalCallEdges,
],
None,
pm::Optimizations::Allowed,

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.

This is a behavior change I think? Previously SimplifyCfg::MakeShim was always run, now it is controlled by #[optimize].

@@ -479,7 +478,6 @@
&mut body,
&[&promote_pass, &simplify::SimplifyCfg::PromoteConsts, &coverage::InstrumentCoverage],

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.

Another SimplifyCfg that's newly controlled by the attribute.

body,
&[
&remove_uninit_drops::RemoveUninitDrops,
&simplify::SimplifyCfg::RemoveFalseEdges,

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.

And another one.

&impossible_predicates::ImpossiblePredicates,
&cleanup_post_borrowck::CleanupPostBorrowck,
&remove_noop_landing_pads::RemoveNoopLandingPads,
&simplify::SimplifyCfg::PostAnalysis,

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.

And another one

@@ -682,13 +673,7 @@
&simplify::SimplifyCfg::PreOptimizations,

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.

and another one

@RalfJung

RalfJung commented Jul 28, 2026

Copy link
Copy Markdown
Member

meta-pondering: Does optimize(none) really want to be different from opt-level=0?

You mean -Copt-level=0?

Yeah TBH I tend to agree... I feel like optimize would ideally be implemented by overwriting the mir_opt_level that passes use as the basis for their decision. (I don't know if any MIR opts do anything with -Copt-level=s/z.)

Should maybe we stop checking mir-opt-level from the session in passes and instead check something passed-in only, with that passed-in thing merging the various knobs into one?

👍

And level 0 should then IMO mean "preserve UB" so we don't need a separate knob for that either.

... though that makes it tricky to have a pass that optionally can preserve UB, since that pass will check the mir-opt-level at pass execution time rather than in is_enabled/policy.

@RalfJung RalfJung left a comment

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.

Independent of that I think this PR is an improvement, so r=me if what I wrote about the SimplifyCfg changes sounds right.

View changes since this review

@clubby789

Copy link
Copy Markdown
Contributor Author

I agree that all the SimplifyCfg passes should be controlled by the attribute
@bors=RalfJung

@rust-bors

rust-bors Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Unknown command "". Run @bors help or go to https://bors.rust-lang.org/help to see available commands.

@RalfJung

Copy link
Copy Markdown
Member

@bors r=RalfJung

@rust-bors

rust-bors Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

📌 Commit a6b2d2b has been approved by RalfJung

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2026
rust-bors Bot pushed a commit that referenced this pull request Jul 28, 2026
…uwer

Rollup of 21 pull requests

Successful merges:

 - #159990 (Many "predicate"-to-"clause" renamings)
 - #159665 (Replace most `Ty::new_fn_def` calls with `type_of` queries directly)
 - #159687 (rustdoc: Set tracing max_level_info when debug-logging is false)
 - #160057 (refactor(mir-transform): Calculate optimization status inside `run_passes_inner`)
 - #160060 (codegen: skip stores for entirely-uninit constant aggregate fields, attempt #2)
 - #160063 (Fix ICE when dumping the dep graph with the parallel frontend)
 - #160065 (Distinguish the dep-graph index space from the live node count)
 - #158038 (Split register_tool into register_attribute_tool and register_lint_tool)
 - #159776 (remove const hack in alloc)
 - #159978 (run intrinsic-test by default on x86_64-gnu)
 - #160008 (Avoid stale closure recovery state across statements)
 - #160027 (Add regression test for #132767)
 - #160030 (Update `browser-ui-test` version to `0.25.0`)
 - #160046 (Improve consistency of attribute error messages (part 2))
 - #160056 (Fix associated function suggestion for generic ADTs)
 - #160069 (Update Rust crate tracing-subscriber to v0.3.23 [SECURITY])
 - #160071 (sanitize_standard_fds: clarify macos comment)
 - #160076 (use unstable features when updating dependencies)
 - #160092 (miri ui tests: don't run native tests on stage 0)
 - #160093 (Switch cargo assignments to weihanglo)
 - #160094 (Update assignment for docs)
@rust-bors
rust-bors Bot merged commit 3048347 into rust-lang:main Jul 28, 2026
13 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 28, 2026
rust-timer added a commit that referenced this pull request Jul 28, 2026
Rollup merge of #160057 - clubby789:local-optimizations, r=RalfJung

refactor(mir-transform): Calculate optimization status inside `run_passes_inner`

The `Optimizations` enum represents whether `#[optimize(none)]` has been applied to a function. Currently, `run_passes` takes an `Optimizations` which is confusing (#160015 (comment)).
Calculate this in `run_passes_inner` instead (which also means `#[optimize(none)]` is applied more consistently.

r? @RalfJung
pull Bot pushed a commit to LeeeeeeM/miri that referenced this pull request Jul 29, 2026
…uwer

Rollup of 21 pull requests

Successful merges:

 - rust-lang/rust#159990 (Many "predicate"-to-"clause" renamings)
 - rust-lang/rust#159665 (Replace most `Ty::new_fn_def` calls with `type_of` queries directly)
 - rust-lang/rust#159687 (rustdoc: Set tracing max_level_info when debug-logging is false)
 - rust-lang/rust#160057 (refactor(mir-transform): Calculate optimization status inside `run_passes_inner`)
 - rust-lang/rust#160060 (codegen: skip stores for entirely-uninit constant aggregate fields, attempt rust-lang/rust#2)
 - rust-lang/rust#160063 (Fix ICE when dumping the dep graph with the parallel frontend)
 - rust-lang/rust#160065 (Distinguish the dep-graph index space from the live node count)
 - rust-lang/rust#158038 (Split register_tool into register_attribute_tool and register_lint_tool)
 - rust-lang/rust#159776 (remove const hack in alloc)
 - rust-lang/rust#159978 (run intrinsic-test by default on x86_64-gnu)
 - rust-lang/rust#160008 (Avoid stale closure recovery state across statements)
 - rust-lang/rust#160027 (Add regression test for rust-lang/rust#132767)
 - rust-lang/rust#160030 (Update `browser-ui-test` version to `0.25.0`)
 - rust-lang/rust#160046 (Improve consistency of attribute error messages (part 2))
 - rust-lang/rust#160056 (Fix associated function suggestion for generic ADTs)
 - rust-lang/rust#160069 (Update Rust crate tracing-subscriber to v0.3.23 [SECURITY])
 - rust-lang/rust#160071 (sanitize_standard_fds: clarify macos comment)
 - rust-lang/rust#160076 (use unstable features when updating dependencies)
 - rust-lang/rust#160092 (miri ui tests: don't run native tests on stage 0)
 - rust-lang/rust#160093 (Switch cargo assignments to weihanglo)
 - rust-lang/rust#160094 (Update assignment for docs)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants