Skip to content

Clarify the role of the PreCodegen dummy pass#156358

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

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

Conversation

@Zalathar
Copy link
Copy Markdown
Member

@Zalathar Zalathar commented May 9, 2026

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
Copy link
Copy Markdown
Collaborator

rustbot commented May 9, 2026

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

@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 May 9, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 9, 2026

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
Copy link
Copy Markdown
Member

mati865 commented May 11, 2026

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
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 11, 2026

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

@rustbot rustbot assigned oli-obk and unassigned mati865 May 11, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 11, 2026

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 added 2 commits May 11, 2026 23:24
@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
Copy link
Copy Markdown
Collaborator

rustbot commented May 11, 2026

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 thread compiler/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,
Copy link
Copy Markdown
Contributor

@oli-obk oli-obk May 11, 2026 •
edited by rustbot
Loading

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
Copy link
Copy Markdown
Contributor

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

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

@rustbot rustbot 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
Copy link
Copy Markdown
Contributor

cjgillot commented May 11, 2026

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

@Zalathar
Copy link
Copy Markdown
Member Author

Zalathar commented May 12, 2026

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.

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

Reviewers

@oli-obk oli-obk oli-obk requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@oli-obk oli-obk

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant 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