Skip to content

Introduce TypingMode::Codegen to avoid layout cycles on coroutines#145477

Merged
rust-bors[bot] merged 5 commits into
rust-lang:mainfrom
cjgillot:codegen-mode
Jun 4, 2026
Merged

Introduce TypingMode::Codegen to avoid layout cycles on coroutines#145477
rust-bors[bot] merged 5 commits into
rust-lang:mainfrom
cjgillot:codegen-mode

Conversation

@cjgillot
Copy link
Copy Markdown
Contributor

@cjgillot cjgillot commented Aug 16, 2025

View all comments

Computing layout of coroutines depends on their optimized_mir. At the same time, MIR opts can require using layouts to work. For instance to evaluate constants. This leads to cycles and clumsy workarounds.

This PR creates a new typing mode for layout computations:

  • when a coroutine's layout is requested with TypingMode::PostAnalysis or earlier, return LayourError::TooGeneric;
  • when a coroutine's layout is requested with TypingMode::Codegen, actually compute it.

TypingMode::Codegen is meant be be used by codegen code, and analyses that require coroutine layout, like transmute check and coroutine recursion check.

With this PR, we can remove all is_coroutine checks from rustc_mir_transform and unlock simplifying coroutine MIR.

Perf is not terrific. This PR causes recomputation of a few queries, and I had to insert workarounds.

@rustbot rustbot added 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 16, 2025
@cjgillot
Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors Bot added a commit that referenced this pull request Aug 16, 2025
[EXPERIMENT] Introduce `TypingMode::Codegen` to avoid layout cycles
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 16, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Aug 16, 2025

☀️ Try build successful (CI)
Build commit: 34dd67f (34dd67f42f0f2050c773b332e2dd333618c22743, parent: cd7cbe818e4a66d46fe2df993d1b8518eba8a5cd)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (34dd67f): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.5% [0.2%, 3.3%] 58
Regressions ❌
(secondary)
1.8% [0.0%, 19.4%] 58
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.3%, -0.0%] 4
All ❌✅ (primary) 0.5% [0.2%, 3.3%] 58

Max RSS (memory usage)

Results (primary 1.4%, secondary 4.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.7% [0.5%, 7.3%] 17
Regressions ❌
(secondary)
5.4% [1.2%, 12.3%] 6
Improvements ✅
(primary)
-1.3% [-1.4%, -1.3%] 2
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) 1.4% [-1.4%, 7.3%] 19

Cycles

Results (primary 2.9%, secondary 4.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.9% [2.9%, 2.9%] 1
Regressions ❌
(secondary)
5.3% [2.1%, 12.0%] 12
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.0% [-4.0%, -4.0%] 1
All ❌✅ (primary) 2.9% [2.9%, 2.9%] 1

Binary size

Results (primary 0.1%, secondary -1.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.4% [0.1%, 0.6%] 2
Regressions ❌
(secondary)
0.3% [0.1%, 0.4%] 3
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 3
Improvements ✅
(secondary)
-1.8% [-7.1%, -0.1%] 5
All ❌✅ (primary) 0.1% [-0.0%, 0.6%] 5

Bootstrap: 469.562s -> 472.464s (0.62%)
Artifact size: 377.55 MiB -> 377.67 MiB (0.03%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 16, 2025
@cjgillot
Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors Bot added a commit that referenced this pull request Aug 16, 2025
[EXPERIMENT] Introduce `TypingMode::Codegen` to avoid layout cycles
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 16, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Aug 16, 2025

☀️ Try build successful (CI)
Build commit: 4e9488c (4e9488c147d4057600076145eccb670b130147d5, parent: 2e2642e641a941f0a1400c7827fd89aa86fef8f4)

@rust-timer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (4e9488c): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.4% [0.2%, 0.9%] 23
Regressions ❌
(secondary)
1.5% [0.1%, 9.7%] 59
Improvements ✅
(primary)
-1.6% [-1.6%, -1.6%] 1
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 2
All ❌✅ (primary) 0.4% [-1.6%, 0.9%] 24

Max RSS (memory usage)

Results (primary 1.7%, secondary 5.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.9% [0.5%, 7.5%] 19
Regressions ❌
(secondary)
6.6% [0.4%, 12.9%] 5
Improvements ✅
(primary)
-1.3% [-1.3%, -1.3%] 1
Improvements ✅
(secondary)
-2.8% [-2.8%, -2.8%] 1
All ❌✅ (primary) 1.7% [-1.3%, 7.5%] 20

Cycles

Results (secondary 2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.2% [2.1%, 10.2%] 8
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% [-3.8%, -2.2%] 2
All ❌✅ (primary) - - 0

Binary size

Results (primary 0.0%, secondary -1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.1% [0.1%, 0.1%] 1
Regressions ❌
(secondary)
0.2% [0.1%, 0.4%] 4
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 3
Improvements ✅
(secondary)
-2.9% [-6.8%, -0.0%] 3
All ❌✅ (primary) 0.0% [-0.0%, 0.1%] 4

Bootstrap: 470.222s -> 472.094s (0.40%)
Artifact size: 377.51 MiB -> 377.42 MiB (-0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 16, 2025
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Copy Markdown
Collaborator

bors commented Aug 23, 2025

☔ The latest upstream changes (presumably #145469) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@oli-obk
Copy link
Copy Markdown
Contributor

oli-obk commented Jun 3, 2026

r=me (needs rebase)

@oli-obk oli-obk 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 Jun 3, 2026
@oli-obk
Copy link
Copy Markdown
Contributor

oli-obk commented Jun 3, 2026

#145477 (review)

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jun 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 3, 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.

@cjgillot
Copy link
Copy Markdown
Contributor Author

cjgillot commented Jun 3, 2026

@bors r=oli-obk

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 3, 2026

📌 Commit ac71b54 has been approved by oli-obk

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 3, 2026
@rust-bors

This comment has been minimized.

@rust-bors rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 4, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 4, 2026

☀️ Test successful - CI
Approved by: oli-obk
Duration: 3h 13m 59s
Pushing 76dfce2 to main...

@rust-bors rust-bors Bot merged commit 76dfce2 into rust-lang:main Jun 4, 2026
13 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing b354133 (parent) -> 76dfce2 (this PR)

Test differences

Show 12 test diffs

12 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 76dfce2cb2d3f7b7f34d62e6ffe044f7e7d76948 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-apple-various: 1h 25m -> 2h 16m (+59.5%)
  2. dist-x86_64-msvc-alt: 2h 50m -> 1h 40m (-41.1%)
  3. x86_64-gnu-miri: 1h 11m -> 1h 38m (+37.3%)
  4. x86_64-mingw-1: 2h 54m -> 1h 50m (-36.4%)
  5. dist-armv7-linux: 1h 31m -> 59m 29s (-35.3%)
  6. dist-aarch64-apple: 2h 12m -> 1h 26m (-34.3%)
  7. x86_64-gnu-distcheck: 1h 51m -> 2h 26m (+30.8%)
  8. x86_64-msvc-2: 2h 42m -> 1h 55m (-29.0%)
  9. dist-loongarch64-musl: 1h 27m -> 1h 50m (+26.7%)
  10. x86_64-msvc-1: 2h 1m -> 2h 30m (+24.6%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (76dfce2): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.5% [0.1%, 1.1%] 51
Regressions ❌
(secondary)
1.5% [0.1%, 9.9%] 43
Improvements ✅
(primary)
-0.5% [-0.5%, -0.5%] 1
Improvements ✅
(secondary)
-0.4% [-0.4%, -0.4%] 1
All ❌✅ (primary) 0.4% [-0.5%, 1.1%] 52

Max RSS (memory usage)

Results (primary 1.2%, secondary -1.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.6% [0.7%, 2.8%] 13
Regressions ❌
(secondary)
2.8% [2.8%, 2.8%] 1
Improvements ✅
(primary)
-1.0% [-1.4%, -0.6%] 2
Improvements ✅
(secondary)
-2.9% [-8.2%, -1.0%] 4
All ❌✅ (primary) 1.2% [-1.4%, 2.8%] 15

Cycles

Results (secondary 1.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.3% [2.1%, 6.6%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.4% [-2.6%, -2.2%] 2
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 511.076s -> 511.359s (0.06%)
Artifact size: 400.68 MiB -> 400.66 MiB (-0.01%)

@cjgillot cjgillot deleted the codegen-mode branch June 4, 2026 08:07
@lcnr
Copy link
Copy Markdown
Contributor

lcnr commented Jun 4, 2026

two thoughts, this feels related to the addition of TypingMode::ErasedNonCoherence, i.e. the trait solver mostly doesn't care which typingmode we're in between these two post-analysis modes, so we can probably extend this Erased behavior to also handle different codegen modes if perf requires it

the other one is that I am vaguely unhappy about adding new TypingMode. They have a fairly high complexity cost and I want to really stay on top of the ones we add.

I don't think this PR is necessarily wrong, I would like this to have gone through a types MCP or at least nomination.

Comment on lines +32 to +41
// We are in codegen. It's very likely this constant has been evaluated in PostAnalysis
// before. Try to reuse this evaluation, and only re-run if we hit a `TooGeneric` error.
ty::TypingMode::Codegen => {
let with_postanalysis =
ty::TypingEnv::new(typing_env.param_env, ty::TypingMode::PostAnalysis);
let with_postanalysis = f(with_postanalysis.as_query_input(value));
match with_postanalysis {
Ok(_) | Err(ErrorHandled::Reported(..)) => return Some(with_postanalysis),
Err(ErrorHandled::TooGeneric(_)) => {}
}
Copy link
Copy Markdown
Member

@nbdd0121 nbdd0121 Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fact that this exists make me feel that this is not the right approach.

Perhaps we should just not use optimized_mir for coroutine layout computation? Coud we just run a fixed list of passes that does not require knowing layouts for coroutine layout computation?

View changes since the review

nbdd0121 added a commit to Rust-for-Linux/klint that referenced this pull request Jun 5, 2026
Upstream rustc changes [1] `fully_monomophized` to `TypingMode::Codegen`
instead of `TypingMode::post_analysis`. This causes many queries to run
multiple times and cause issues because these modes do not compare equal.

Switch everything to use `TypingEnv::codegen` to still have a single typing
env shared by all analysis.

Link: rust-lang/rust#145477 [1]
Signed-off-by: Gary Guo <gary@garyguo.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants