Skip to content

Run RemoveZsts pass at mir-opt-level=1 #83417

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

Merged
merged 1 commit into from
Aug 14, 2021
Merged

Run RemoveZsts pass at mir-opt-level=1 #83417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions compiler/rustc_mir/src/transform/remove_zsts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ pub struct RemoveZsts;

impl<'tcx> MirPass<'tcx> for RemoveZsts {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.mir_opt_level() < 3 {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be kept and use < 1 instead? I noticed that some const errors have changed, so I'm wondering if it's being triggered on mir-opt-level=0.

Copy link
Contributor Author

@erikdesjardins erikdesjardins Mar 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It doesn't run at mir-opt-level=0.

See

rust/compiler/rustc_mir/src/transform/mod.rs

Line 498 in 79e5814

&remove_zsts::RemoveZsts,
and

rust/compiler/rustc_mir/src/transform/mod.rs

Line 558 in 79e5814

if mir_opt_level > 0 { optimizations } else { no_optimizations },

The error changes because it removes an assignment, so we hit a different error path due to a different use of the constant. The specific error messages changed in this PR are fine (see #83177 (comment)).

But this area seems undertested, I think I've found some cases where it's wrong. I'll open another PR to add tests, that we should merge before this one.

Copy link
Member

Choose a reason for hiding this comment

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

The error changes because it removes an assignment, so we hit a different error path due to a different use of the constant. The specific error messages changed in this PR are fine (see #83177 (comment)).

I see, does the mir-opt-level apply to const-eval as well?

Copy link
Contributor Author

@erikdesjardins erikdesjardins Mar 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I believe so, because I believe the same MIR is used for const eval and codegen.

But I don't think that's relevant here--the error that changes is in main, not in const eval'd code.

There is a separate const eval error due to the panic (reported as a future compat warning):

rust/src/test/ui/consts/const-eval/panic-never-type.rs

Lines 8 to 10 in 79e5814

const VOID: ! = panic!();
//~^ WARN any use of this value will cause an error
//~| WARN this was previously accepted by the compiler but is being phased out

Copy link
Contributor

Choose a reason for hiding this comment

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

Since #78407, CTFE uses unoptimized MIR to avoid situations where an undefined behaviour is hidden because of MIR optimizations.

Copy link
Member

Choose a reason for hiding this comment

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

But I don't think that's relevant here--the error that changes is in main, not in const eval'd code.

Ah, I didn't notice that. Thanks for clearing up my confusion :)

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for the change in diagnostics is that const propagation now doesn't see the constant anymore, but codegen will still evaluate it because it was used in the initial MIR body (see the https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir/transform/required_consts/index.html field)

Copy link
Contributor

Choose a reason for hiding this comment

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

Another way to resolve that these tests get changed would be to eagerly evaluate all required_consts in const propagation

To stop depending on optimizations reporting these entirely, we could also evaluate the constant in

rust/compiler/rustc_mir/src/transform/required_consts.rs

Line 18 in 372afcf

if let ConstKind::Unevaluated(_) = ct.val {
and only put it into required_consts if the evaluation did not succeed

return;
}
let param_env = tcx.param_env(body.source.def_id());
let (basic_blocks, local_decls) = body.basic_blocks_and_local_decls_mut();
for block in basic_blocks.iter_mut() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/incremental/hashes/trait_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl ChangeMethodBodyTrait for Foo {
#[rustc_clean(cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
impl ChangeMethodBodyTrait for Foo {
#[rustc_clean(except="hir_owner_nodes,typeck,optimized_mir", cfg="cfail2")]
#[rustc_clean(except="hir_owner_nodes,typeck", cfg="cfail2")]
#[rustc_clean(cfg="cfail3")]
fn method_name() {
()
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/control_flow_simplification.hello.ConstProp.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
}

bb2: {
_0 = const (); // scope 0 at $DIR/control-flow-simplification.rs:14:6: 14:6
nop; // scope 0 at $DIR/control-flow-simplification.rs:14:6: 14:6
StorageDead(_1); // scope 0 at $DIR/control-flow-simplification.rs:14:5: 14:6
return; // scope 0 at $DIR/control-flow-simplification.rs:15:2: 15:2
}
Expand Down
1 change: 0 additions & 1 deletion src/test/mir-opt/const_prop/control_flow_simplification.hello.PreCodegen.before.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ fn hello() -> () {
let mut _0: (); // return place in scope 0 at $DIR/control-flow-simplification.rs:11:14: 11:14

bb0: {
_0 = const (); // scope 0 at $DIR/control-flow-simplification.rs:14:6: 14:6
return; // scope 0 at $DIR/control-flow-simplification.rs:15:2: 15:2
}
}
1 change: 0 additions & 1 deletion src/test/mir-opt/remove_unneeded_drops.cannot_opt_generic.RemoveUnneededDrops.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
_3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
nop; // scope 1 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
}

Expand Down
1 change: 0 additions & 1 deletion src/test/mir-opt/remove_unneeded_drops.dont_opt.RemoveUnneededDrops.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
_3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
nop; // scope 1 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
}

Expand Down
1 change: 0 additions & 1 deletion src/test/mir-opt/remove_unneeded_drops.opt.RemoveUnneededDrops.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
_3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
- nop; // scope 1 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
- drop(_3) -> bb1; // scope 1 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
- }
-
Expand Down
1 change: 0 additions & 1 deletion src/test/mir-opt/remove_unneeded_drops.opt_generic_copy.RemoveUnneededDrops.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
_3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
- nop; // scope 1 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
- drop(_3) -> bb1; // scope 1 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
- }
-
Expand Down
49 changes: 15 additions & 34 deletions src/test/mir-opt/simplify_locals_fixedpoint.foo.SimplifyLocals.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
let mut _4: isize; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:22: 4:26
let mut _5: isize; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:13: 4:20
let _6: u8; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:18: 4:19
let mut _7: bool; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:20
let mut _8: u8; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:13
- let mut _7: bool; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:20
- let mut _8: u8; // in scope 0 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:13
scope 1 {
debug a => _6; // in scope 1 at $DIR/simplify-locals-fixedpoint.rs:4:18: 4:19
}
Expand All @@ -26,51 +26,32 @@
StorageDead(_3); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:68: 4:69
StorageDead(_2); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:68: 4:69
_5 = discriminant((_1.0: std::option::Option<u8>)); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:13: 4:20
switchInt(move _5) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:13: 4:20
switchInt(move _5) -> [1_isize: bb1, otherwise: bb3]; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:13: 4:20
}

bb1: {
_0 = const (); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:8:6: 8:6
goto -> bb7; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:5: 8:6
}

bb2: {
_4 = discriminant((_1.1: std::option::Option<T>)); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:22: 4:26
switchInt(move _4) -> [0_isize: bb3, otherwise: bb1]; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:22: 4:26
switchInt(move _4) -> [0_isize: bb2, otherwise: bb3]; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:22: 4:26
}

bb3: {
bb2: {
StorageLive(_6); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:18: 4:19
_6 = (((_1.0: std::option::Option<u8>) as Some).0: u8); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:18: 4:19
StorageLive(_7); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:20
StorageLive(_8); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:13
_8 = _6; // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:13
_7 = Gt(move _8, const 42_u8); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:20
StorageDead(_8); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:19: 5:20
switchInt(move _7) -> [false: bb5, otherwise: bb4]; // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:9: 7:10
}

bb4: {
_0 = const (); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:21: 7:10
goto -> bb6; // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:9: 7:10
}

bb5: {
_0 = const (); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:7:10: 7:10
goto -> bb6; // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:9: 7:10
}

bb6: {
StorageDead(_7); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:7:9: 7:10
- StorageLive(_7); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:20
- StorageLive(_8); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:13
- _8 = _6; // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:13
- _7 = Gt(move _8, const 42_u8); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:12: 5:20
- StorageDead(_8); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:5:19: 5:20
- StorageDead(_7); // scope 1 at $DIR/simplify-locals-fixedpoint.rs:7:9: 7:10
StorageDead(_6); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:8:5: 8:6
goto -> bb7; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:5: 8:6
goto -> bb3; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:4:5: 8:6
}

bb7: {
drop(_1) -> bb8; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:9:1: 9:2
bb3: {
drop(_1) -> bb4; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:9:1: 9:2
}

bb8: {
bb4: {
StorageDead(_1); // scope 0 at $DIR/simplify-locals-fixedpoint.rs:9:1: 9:2
return; // scope 0 at $DIR/simplify-locals-fixedpoint.rs:9:2: 9:2
}
Expand Down
1 change: 0 additions & 1 deletion src/test/run-make/const_fn_mir/dump.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ fn main() -> () {
}

bb1: {
_0 = const (); // scope 0 at main.rs:8:11: 10:2
return; // scope 0 at main.rs:10:2: 10:2
}
}
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/consts/const-eval/panic-assoc-never-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,4 @@ impl PrintName {

fn main() {
let _ = PrintName::VOID;
//~^ ERROR erroneous constant used
}
8 changes: 1 addition & 7 deletions src/test/ui/consts/const-eval/panic-assoc-never-type.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ LL | const VOID: ! = panic!();
|
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: erroneous constant used
--> $DIR/panic-assoc-never-type.rs:16:13
|
LL | let _ = PrintName::VOID;
| ^^^^^^^^^^^^^^^ referenced constant has errors

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
6 changes: 3 additions & 3 deletions src/test/ui/recursion/issue-26548-recursion-via-normalize.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
error[E0391]: cycle detected when computing layout of `S`
error[E0391]: cycle detected when computing layout of `std::option::Option<S>`
|
= note: ...which requires computing layout of `std::option::Option<S>`...
= note: ...which again requires computing layout of `S`, completing the cycle
= note: ...which requires computing layout of `S`...
= note: ...which again requires computing layout of `std::option::Option<S>`, completing the cycle
note: cycle used when optimizing MIR for `main`
--> $DIR/issue-26548-recursion-via-normalize.rs:15:1
|
Expand Down