Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix ConstProp handling of written_only_inside_own_block_locals #102045

Merged
merged 1 commit into from
Sep 21, 2022
Merged

fix ConstProp handling of written_only_inside_own_block_locals #102045

merged 1 commit into from
Sep 21, 2022

Conversation

RalfJung
Copy link
Member

Fixes a regression introduced by #100239, which adds an early return and thus skips some code in visit_terminator that must be run for soundness.

Fixes #101973

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 20, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 20, 2022

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-highfive
Copy link
Collaborator

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2022
@RalfJung RalfJung mentioned this pull request Sep 20, 2022
Closed
oli-obk
oli-obk reviewed Sep 20, 2022
View reviewed changes
compiler/rustc_mir_transform/src/const_prop.rs
@@ -1066,32 +1066,32 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
let source_info = terminator.source_info;
self.source_info = Some(source_info);
self.super_terminator(terminator, location);
// Do NOT early return in this function, it does some crucial fixup of the state at the end!
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably put the non-fixup logic into a separate function that can then early return...

Copy link
Member Author

@RalfJung RalfJung Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There's a lot of refactoring that could be done here... by someone who actually understands that code.^^

(I also still haven't quite figured out what check_binary_op actually checks and what its return value means. Few of the functions in this file have doc comments.)

@oli-obk
Copy link
Contributor

oli-obk commented Sep 20, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Sep 20, 2022

📌 Commit 7373788 has been approved by oli-obk

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 20, 2022
@RalfJung
Copy link
Member Author

@Mark-Simulacrum this should probably be backported. Should this PR be beta-nominated and then someone does the backporting, or should I open a backported PR?

@oli-obk oli-obk added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 20, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 21, 2022
@Dylan-DPC
Rollup merge of rust-lang#102045 - RalfJung:const-prop-regression-fix…
382f31a 
…, r=oli-obk

fix ConstProp handling of written_only_inside_own_block_locals

Fixes a regression introduced by rust-lang#100239, which adds an early return and thus skips some code in `visit_terminator` that must be run for soundness.

Fixes rust-lang#101973
This was referenced Sep 21, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2022
@bors
Auto merge of rust-lang#102097 - Dylan-DPC:rollup-gc75oh4, r=Dylan-DPC
db4b4d3 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#89891 (`alloc`: add unstable cfg features `no_rc` and `no_sync`)
 - rust-lang#101995 (Add another example for `uN::carrying_mul`)
 - rust-lang#102031 (Adding ignore fuchsia tests for Backtrace, ErrorKind cases)
 - rust-lang#102041 (Improve `-Zmeta-stats` some more)
 - rust-lang#102045 (fix ConstProp handling of written_only_inside_own_block_locals)
 - rust-lang#102058 (Clarify Path::extension() semantics in docs abstract)
 - rust-lang#102059 (Use rebind instead of dummy binder in `SameTypeModuloInfer` relation)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7d7f555 into rust-lang:master Sep 21, 2022
@rustbot rustbot added this to the 1.66.0 milestone Sep 21, 2022
@RalfJung RalfJung deleted the const-prop-regression-fix branch September 21, 2022 20:06
@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 22, 2022
@cuviper cuviper mentioned this pull request Oct 4, 2022
Merged
@cuviper cuviper modified the milestones: 1.66.0, 1.65.0 Oct 4, 2022
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 4, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2022
@bors
Auto merge of rust-lang#102676 - cuviper:beta-next, r=cuviper
da7ffa2 
[beta] backports

* Avoid duplicating StorageLive in let-else rust-lang#101894
* Re-add HRTB implied static bug note rust-lang#101924
* Revert "Copy stage0 binaries into stage0-sysroot" rust-lang#101942
* implied_bounds: deal with inference vars rust-lang#102016
* fix ConstProp handling of written_only_inside_own_block_locals rust-lang#102045
* Fix wrongly refactored Lift impl rust-lang#102088
* Fix a typo “pararmeter” in error message rust-lang#102119
* Deny associated type bindings within associated type bindings rust-lang#102338
* Continue migration of CSS themes rust-lang#101934
* Fix search result colors rust-lang#102369
* Fix unwind drop glue for if-then scopes rust-lang#102394
* Revert "Use getentropy when possible on all Apple platforms" rust-lang#102693
* Fix associated type bindings with anon const in GAT position rust-lang#102336
* Revert perf-regression 101620 rust-lang#102064
* `EscapeAscii` is not an `ExactSizeIterator` rust-lang#99880
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 left review comments

Assignees

@cjgillot cjgillot

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.

Incorrect code generated by MIR optimization on nightly
8 participants
@RalfJung @rustbot @rust-highfive @oli-obk @bors @apiraino @cuviper @cjgillot