Skip to content

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Sep 27, 2025

Since #128612, we compute optimized MIR when -Zvalidate-mir is present.

This is done as part of required analyses, even if type-checking fails. This causes ICEs, as most of the mir-opt pipeline expects well-formed code.

Fixes #129095
Fixes #134174
Fixes #134654
Fixes #135570
Fixes #136381
Fixes #137468
Fixes #144491
Fixes #147011

This does not fix issue #137190, as it ICEs without -Zvalidate-mir.

r? @compiler-errors

@rustbot
Copy link
Collaborator

rustbot commented Sep 27, 2025

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

@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 Sep 27, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 27, 2025

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

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

@cjgillot
Copy link
Contributor Author

@bors r=compiler-errors rollup

@bors
Copy link
Collaborator

bors commented Sep 27, 2025

📌 Commit c6657be has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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 Sep 27, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 28, 2025
…iler-errors

Do not compute optimized MIR if code does not type-check.

Since rust-lang#128612, we compute optimized MIR when `-Zvalidate-mir` is present.

This is done as part of required analyses, even if type-checking fails. This causes ICEs, as most of the mir-opt pipeline expects well-formed code.

Fixes rust-lang#129095
Fixes rust-lang#134174
Fixes rust-lang#134654
Fixes rust-lang#136381
Fixes rust-lang#137468
Fixes rust-lang#144491
Fixes rust-lang#147011

This does not fix issue rust-lang#137190, as it ICEs without `-Zvalidate-mir`.

r? `@compiler-errors`
bors added a commit that referenced this pull request Sep 28, 2025
Rollup of 16 pull requests

Successful merges:

 - #142139 (Include additional hashes in src/stage0)
 - #146745 (Clarified error note for usize range matching)
 - #146763 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 5))
 - #146788 (chore: removes deprecated discord.)
 - #146942 ([rustdoc] Finish getting rid of usages `write_str`)
 - #147002 (rustdoc-search: stringdex update with more packing)
 - #147061 (fix rebasing cycle heads when not reaching a fixpoint)
 - #147066 (Fix tracking issue number for feature(macro_attr))
 - #147081 (doc: fix a typo in platform-support.md)
 - #147082 (formatting_options: fix alternate docs 0b/0o mixup)
 - #147086 (compiletest: Use `PanicHookInfo::payload_as_str` now that it's stable in beta)
 - #147092 (Do not compute optimized MIR if code does not type-check.)
 - #147093 (redox: switch to colon as path separator)
 - #147095 (Library: Remove remaining private `#[repr]` workarounds)
 - #147098 (Add auto extra-checks in pre-push hook)
 - #147110 (Fix typo)

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar
Copy link
Contributor

Probably failed in rollup due to crash test not crashing: #147114 (comment)

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 28, 2025
@Zalathar
Copy link
Contributor

(Bisecting the rollup now; I'll reapprove if my guess was wrong.)

@Zalathar
Copy link
Contributor

Zalathar commented Sep 28, 2025

OK I can confirm that this PR also fixes tests/crashes/134654.rs and tests/crashes/135570.rs.

Tracking this down was made much harder by the tests being marked //@ only-x86_64 for seemingly no reason.

@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2025

This PR was rebased onto a different master 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
Contributor Author

@bors r=compiler-errors rollup

@bors
Copy link
Collaborator

bors commented Sep 28, 2025

📌 Commit 7a7cb05 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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 Sep 28, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 29, 2025
…iler-errors

Do not compute optimized MIR if code does not type-check.

Since rust-lang#128612, we compute optimized MIR when `-Zvalidate-mir` is present.

This is done as part of required analyses, even if type-checking fails. This causes ICEs, as most of the mir-opt pipeline expects well-formed code.

Fixes rust-lang#129095
Fixes rust-lang#134174
Fixes rust-lang#134654
Fixes rust-lang#135570
Fixes rust-lang#136381
Fixes rust-lang#137468
Fixes rust-lang#144491
Fixes rust-lang#147011

This does not fix issue rust-lang#137190, as it ICEs without `-Zvalidate-mir`.

r? `@compiler-errors`
bors added a commit that referenced this pull request Sep 29, 2025
Rollup of 6 pull requests

Successful merges:

 - #133477 (Detect tuple structs that are unconstructable due to re-export)
 - #140916 (Fix unuseful span in type error in some format_args!() invocations)
 - #146979 (constify Default on Nanoseconds)
 - #147090 (Skip stack overflow handler for panic=immediate-abort)
 - #147092 (Do not compute optimized MIR if code does not type-check.)
 - #147127 (Add a leading dash to linker plugin arguments in the gcc codegen)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 29, 2025
Rollup of 7 pull requests

Successful merges:

 - #133477 (Detect tuple structs that are unconstructable due to re-export)
 - #146929 (compiletest: Remove old-output-capture and become a stage0 bootstrap tool)
 - #146979 (constify Default on Nanoseconds)
 - #147092 (Do not compute optimized MIR if code does not type-check.)
 - #147112 (all 48 keywords in just 300 characters)
 - #147122 (Fix some crash-test directives)
 - #147127 (Add a leading dash to linker plugin arguments in the gcc codegen)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 01b172e into rust-lang:master Sep 29, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 29, 2025
rust-timer added a commit that referenced this pull request Sep 29, 2025
Rollup merge of #147092 - cjgillot:late-validate-mir, r=compiler-errors

Do not compute optimized MIR if code does not type-check.

Since #128612, we compute optimized MIR when `-Zvalidate-mir` is present.

This is done as part of required analyses, even if type-checking fails. This causes ICEs, as most of the mir-opt pipeline expects well-formed code.

Fixes #129095
Fixes #134174
Fixes #134654
Fixes #135570
Fixes #136381
Fixes #137468
Fixes #144491
Fixes #147011

This does not fix issue #137190, as it ICEs without `-Zvalidate-mir`.

r? ``@compiler-errors``
@cjgillot cjgillot deleted the late-validate-mir branch September 29, 2025 10:26
@Zalathar
Copy link
Contributor

@rust-timer build 5e08e7f

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5e08e7f): 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)
3.0% [3.0%, 3.0%] 1
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.0% [3.0%, 3.0%] 1

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (primary 2.3%)

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

mean range count
Regressions ❌
(primary)
2.3% [1.9%, 2.7%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.3% [1.9%, 2.7%] 2

Binary size

Results (primary 1.1%)

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

mean range count
Regressions ❌
(primary)
1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.1% [1.1%, 1.1%] 1

Bootstrap: 469.945s -> 471.627s (0.36%)
Artifact size: 387.67 MiB -> 387.67 MiB (0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment