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

@cjgillotcjgillot commented Aug 16, 2025

Copy link
Copy Markdown
Contributor

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.

@rustbotrustbot 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
ContributorAuthor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-borsBot 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.

@rustbotrustbot 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

rust-borsBot commented Aug 16, 2025

Copy link
Copy Markdown
Contributor

☀️ 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.

meanrangecount
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.

meanrangecount
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.

meanrangecount
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.

meanrangecount
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%)

@rustbotrustbot 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
ContributorAuthor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-borsBot added a commit that referenced this pull request Aug 16, 2025
[EXPERIMENT] Introduce `TypingMode::Codegen` to avoid layout cycles
@rustbotrustbot 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

rust-borsBot commented Aug 16, 2025

Copy link
Copy Markdown
Contributor

☀️ 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.

meanrangecount
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.

meanrangecount
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.

meanrangecount
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.

meanrangecount
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%)

@rustbotrustbot 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

bors commented Aug 23, 2025

Copy link
Copy Markdown
Collaborator

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

@rust-log-analyzer

This comment has been minimized.

@rustbot

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.

@cjgillot

Copy link
Copy Markdown
ContributorAuthor

@bors r=oli-obk

@rust-bors

rust-borsBot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

📌 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-borsrust-borsBot 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-borsrust-borsBot 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

rust-borsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

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

@rust-bors
rust-borsBot merged commit 76dfce2 into rust-lang:mainJun 4, 2026
13 checks passed
@rustbotrustbot added this to the 1.98.0 milestone Jun 4, 2026
@github-actions

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

meanrangecount
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.

meanrangecount
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.

meanrangecount
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

lcnr commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

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(_)) => {}
}

@nbdd0121nbdd0121Jun 5, 2026

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 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>
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 11, 2026
Introduce `TypingMode::Codegen` to avoid layout cycles on coroutines
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.
@frewsxcv

Copy link
Copy Markdown
Contributor

This fixed an ICE. See: #158328

@danderson

Copy link
Copy Markdown

Just to be a pessimist: it's possible my reproducer on that bug is just fragile, and this change incidentally makes that specific repro avoid the panic codepath. I defer to rustc experts though, I don't have anywhere near the knowledge of rustc internals to evaluate that.

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Aug 23, 2026
With the new trait solver enabled by default in nightly-2026-08-22 [1],
large_futures can ICE while inspecting an awaited future.
The lint switches to Codegen mode to obtain coroutine layouts but passes
a type whose opaque aliases are still marked rigid from type checking.
Because layout_of receives an already-Codegen environment, its
environment-change path does not clear those markers.
Clear alias rigidity and normalize the awaited type in that environment
before querying its layout. Codegen mode must remain: post-analysis
layout queries cannot compute coroutine sizes [2].
The regression covers direct and pinned futures under both solvers,
without other lint errors that could hide a delayed ICE.
Fixesrust-lang/rust#161495.
[1]: rust-lang/rust#160619
[2]: rust-lang/rust#145477
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Aug 23, 2026
With the new trait solver enabled by default in nightly-2026-08-22 [1],
large_futures can ICE while inspecting an awaited future.
The lint switches to Codegen mode to obtain coroutine layouts but passes
a type whose opaque aliases are still marked rigid from type checking.
Because layout_of receives an already-Codegen environment, its
environment-change path does not clear those markers.
Clear alias rigidity and normalize the awaited type in that environment
before querying its layout. Codegen mode must remain: post-analysis
layout queries cannot compute coroutine sizes [2].
The regression covers direct and pinned futures under both solvers,
without other lint errors that could hide a delayed ICE.
Fixesrust-lang/rust#161495.
[1]: rust-lang/rust#160619
[2]: rust-lang/rust#145477
flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Aug 23, 2026
With the new trait solver enabled by default in nightly-2026-08-22 [1],
large_futures can ICE while inspecting an awaited future.
The lint switches to Codegen mode to obtain coroutine layouts but passes
a type whose opaque aliases are still marked rigid from type checking.
Because layout_of receives an already-Codegen environment, its
environment-change path does not clear those markers.
Clear alias rigidity and normalize the awaited type in that environment
before querying its layout. Codegen mode must remain: post-analysis
layout queries cannot compute coroutine sizes [2].
The regression covers direct and pinned futures under both solvers,
without other lint errors that could hide a delayed ICE.
Fixesrust-lang/rust#161495.
changelog: [`large_futures`]: Fix an ICE on awaited futures with the
next trait solver.
[1]: rust-lang/rust#160619
[2]: rust-lang/rust#145477
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-borsThis PR was explicitly merged by bors.perf-regressionPerformance regression.perf-regression-triagedThe performance regression has been triaged.T-clippyRelevant to the Clippy team.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.WG-trait-system-refactorThe Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@cjgillot@rust-timer@rust-log-analyzer@bors@rustbot@oli-obk@lcnr@frewsxcv@danderson@RalfJung@nbdd0121