Skip to content

Clarify the role of the PreCodegen dummy pass - #156358

Closed
Zalathar wants to merge 2 commits into
rust-lang:mainfrom
Zalathar:pre-codegen
Closed

Clarify the role of the PreCodegen dummy pass#156358
Zalathar wants to merge 2 commits into
rust-lang:mainfrom
Zalathar:pre-codegen

Conversation

@Zalathar

@ZalatharZalathar commented May 9, 2026

Copy link
Copy Markdown
Member

The existing comments for this pass are somewhat confusing.

The PreCodegen pass does nothing by itself. It exists so that -Zdump-mir (and mir-opt tests) can easily dump the final pre-codegen MIR, by dumping the input or output of PreCodegen, which happens to be run after all other passes are finished.

@rustbot

rustbot commented May 9, 2026

Copy link
Copy Markdown
Collaborator

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.

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbotrustbot 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 May 9, 2026
@rustbot

rustbot commented May 9, 2026

Copy link
Copy Markdown
Collaborator

r? @mati865

rustbot has assigned @mati865.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, mir, mir-opt
  • compiler, mir, mir-opt expanded to 73 candidates
  • Random selection from 18 candidates

@mati865

mati865 commented May 11, 2026

Copy link
Copy Markdown
Member

I'm not familiar with that area, so the changes look neutral to me.

Perhaps somebody familiar with the area should review it?

@bors r? rust-lang/wg-mir-opt

@rust-bors

rust-borsBot commented May 11, 2026

Copy link
Copy Markdown
Contributor

Unknown command "r?". Run @bors help to see available commands.

@rustbotrustbot assigned oli-obk and unassigned mati865May 11, 2026
@rustbot

rustbot commented May 11, 2026

Copy link
Copy Markdown
Collaborator

Error: Parsing assign command in comment failed: ...'commands.
' | error: specify user to assign to at >| ''...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

@rust-bors

This comment has been minimized.

Sign in to view
@Zalathar
Move emit_mir out of rustc_mir_transform::dump_mir
a99a067
This function doesn't have an obvious home, but there's little reason for it to
be in `rustc_mir_transform`, and having it in `rustc_driver_impl::pretty` at
least puts it near other callers of `write_mir_pretty`.
@Zalathar
Change MIR pass dump_mir::Marker to marker::PreCodegen
e94c943
@rustbot

rustbot commented May 11, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main 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.

oli-obk
oli-obk reviewed May 11, 2026
View reviewed changes
Comment threadcompiler/rustc_mir_transform/src/lib.rs
&dump_mir::Marker("PreCodegen"),
// Dummy pass to allow dumping the final pre-codegen MIR for testing/debugging.
// This must be the final pass!
&marker::PreCodegen,

@oli-obkoli-obkMay 11, 2026•
edited by rustbot
Loading

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.

I think the name should still include somewhere that this pass just dumps

View changes since the review

oli-obk
oli-obk requested changes May 11, 2026
View reviewed changes

@oli-obkoli-obk left a comment •
edited by rustbot
Loading

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.

r=me with naming nit

View changes since this review

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2026
@cjgillot

cjgillot commented May 11, 2026

Copy link
Copy Markdown
Contributor

Is this pass still useful ? We now dump a file called runtime-optimized which should contain the same mir.

@Zalathar

Zalathar commented May 12, 2026

Copy link
Copy Markdown
MemberAuthor

We have a bunch of mir-opt tests that currently mention PreCodegen, but if those can all be converted to use runtime-optimized then we should potentially be able to just remove PreCodegen.

@ZalatharZalathar mentioned this pull request May 13, 2026
Merged
@Zalathar

Zalathar commented May 13, 2026

Copy link
Copy Markdown
MemberAuthor

Based on #156358 (comment) I have filed #156524 as an alternative to this PR. That PR migrates from PreCodegen to runtime-optimized, and then removes PreCodegen entirely.

@oli-obkoli-obk closed this May 13, 2026
@rustbotrustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 13, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request May 13, 2026
@JonathanBrouwer
Rollup merge ofrust-lang#156524- Zalathar:no-pre-codegen, r=oli-obk
c978cbe
Remove the dummy `PreCodegen` mir-opt pass, and use `runtime-optimized` instead
- Alternative to rust-lang#156358.
The `PreCodegen` pass doesn't do anything on its own; it was only serving as a marker to allow `-Zdump-mir` and mir-opt tests to easily dump the final MIR just before codegen.
However, rust-lang#156358 (comment) pointed out that the `runtime-optimized` phase transition should dump the same MIR, so there shouldn't be any need for a separate *PreCodegen* pass.
---
r? oli-obk (or compiler)
@Zalathar
Zalathar deleted the pre-codegen branch May 14, 2026 11:57
pullBot pushed a commit to xtqqczze/rust-lang-miri that referenced this pull request May 17, 2026
@JonathanBrouwer
Rollup merge of #156524 - Zalathar:no-pre-codegen, r=oli-obk
2e8cf4e
Remove the dummy `PreCodegen` mir-opt pass, and use `runtime-optimized` instead
- Alternative to rust-lang/rust#156358.
The `PreCodegen` pass doesn't do anything on its own; it was only serving as a marker to allow `-Zdump-mir` and mir-opt tests to easily dump the final MIR just before codegen.
However, rust-lang/rust#156358 (comment) pointed out that the `runtime-optimized` phase transition should dump the same MIR, so there shouldn't be any need for a separate *PreCodegen* pass.
---
r? oli-obk (or compiler)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obkoli-obkoli-obk requested changes

Assignees

@oli-obkoli-obk

Labels

T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Zalathar@rustbot@mati865@cjgillot@oli-obk