Skip to content

New mir-opt deref_separator #95649

New issue

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

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

Already on GitHub? # to your account

Merged
merged 5 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
68 changes: 68 additions & 0 deletions compiler/rustc_mir_transform/src/deref_separator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use crate::MirPass;
use rustc_middle::mir::patch::MirPatch;
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
pub struct Derefer;

pub fn deref_finder<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let mut patch = MirPatch::new(body);
let (basic_blocks, local_decl) = body.basic_blocks_and_local_decls_mut();
for (block, data) in basic_blocks.iter_enumerated_mut() {
for (i, stmt) in data.statements.iter_mut().enumerate() {
match stmt.kind {
StatementKind::Assign(box (og_place, Rvalue::Ref(region, borrow_knd, place))) => {
for (idx, (p_ref, p_elem)) in place.iter_projections().enumerate() {
if p_elem == ProjectionElem::Deref && !p_ref.projection.is_empty() {
// The type that we are derefing
let ty = p_ref.ty(local_decl, tcx).ty;
let temp = patch.new_temp(ty, stmt.source_info.span);

// Because we are assigning this right before original statement
// we are using index i of statement
let loc = Location { block: block, statement_index: i };
patch.add_statement(loc, StatementKind::StorageLive(temp));

// We are adding current p_ref's projections to our
// temp value
let deref_place =
Place::from(p_ref.local).project_deeper(p_ref.projection, tcx);
patch.add_assign(
loc,
Place::from(temp),
Rvalue::Use(Operand::Move(deref_place)),
);

// We are creating a place by using our temp value's location
// and copying derefed values which we need to create new statement
let temp_place =
Place::from(temp).project_deeper(&place.projection[idx..], tcx);
patch.add_assign(
Copy link
Contributor

Choose a reason for hiding this comment

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

This add call is now not necessary anymore, as the exact same statement is created via mutation below

Copy link
Contributor

Choose a reason for hiding this comment

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

What you can add though is a storagdead statement for the temp. Maybe that'll eliminate the extra cleanup block that showed up in a bunch of tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mir dump looks clean now 😸

Copy link
Contributor

Choose a reason for hiding this comment

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

There are still a bunch of new cleanup blocks showing up. Let's see if a storagedead resolves those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought storagedead would resolve all of those 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, misread the code. Yea, not sure what is going on, i'll investigate

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it's just MirPatch doing that eagerly even if unneeded.

loc,
og_place,
Rvalue::Ref(region, borrow_knd, temp_place),
);

let new_stmt = Statement {
source_info: stmt.source_info,
kind: StatementKind::Assign(Box::new((
og_place,
Rvalue::Ref(region, borrow_knd, temp_place),
))),
};
// Replace current statement with newly created one
*stmt = new_stmt;
}
}
}
_ => (),
}
}
}
patch.apply(body);
}

impl<'tcx> MirPass<'tcx> for Derefer {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
deref_finder(tcx, body);
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ mod const_prop_lint;
mod coverage;
mod deaggregator;
mod deduplicate_blocks;
mod deref_separator;
mod dest_prop;
pub mod dump_mir;
mod early_otherwise_branch;
Expand Down Expand Up @@ -431,6 +432,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
// `Deaggregator` is conceptually part of MIR building, some backends rely on it happening
// and it can help optimizations.
&deaggregator::Deaggregator,
&deref_separator::Derefer,
&Lint(const_prop_lint::ConstProp),
];

Expand Down
60 changes: 60 additions & 0 deletions src/test/mir-opt/derefer_test.main.Derefer.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
- // MIR for `main` before Derefer
+ // MIR for `main` after Derefer

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/derefer_test.rs:2:11: 2:11
let mut _1: (i32, i32); // in scope 0 at $DIR/derefer_test.rs:3:9: 3:14
let mut _3: &mut (i32, i32); // in scope 0 at $DIR/derefer_test.rs:4:22: 4:28
+ let mut _6: &mut (i32, i32); // in scope 0 at $DIR/derefer_test.rs:5:13: 5:26
+ let mut _7: &mut (i32, i32); // in scope 0 at $DIR/derefer_test.rs:6:13: 6:26
scope 1 {
debug a => _1; // in scope 1 at $DIR/derefer_test.rs:3:9: 3:14
let mut _2: (i32, &mut (i32, i32)); // in scope 1 at $DIR/derefer_test.rs:4:9: 4:14
scope 2 {
debug b => _2; // in scope 2 at $DIR/derefer_test.rs:4:9: 4:14
let _4: &mut i32; // in scope 2 at $DIR/derefer_test.rs:5:9: 5:10
scope 3 {
debug x => _4; // in scope 3 at $DIR/derefer_test.rs:5:9: 5:10
let _5: &mut i32; // in scope 3 at $DIR/derefer_test.rs:6:9: 6:10
scope 4 {
debug y => _5; // in scope 4 at $DIR/derefer_test.rs:6:9: 6:10
}
}
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/derefer_test.rs:3:9: 3:14
(_1.0: i32) = const 42_i32; // scope 0 at $DIR/derefer_test.rs:3:17: 3:24
(_1.1: i32) = const 43_i32; // scope 0 at $DIR/derefer_test.rs:3:17: 3:24
StorageLive(_2); // scope 1 at $DIR/derefer_test.rs:4:9: 4:14
StorageLive(_3); // scope 1 at $DIR/derefer_test.rs:4:22: 4:28
_3 = &mut _1; // scope 1 at $DIR/derefer_test.rs:4:22: 4:28
(_2.0: i32) = const 99_i32; // scope 1 at $DIR/derefer_test.rs:4:17: 4:29
(_2.1: &mut (i32, i32)) = move _3; // scope 1 at $DIR/derefer_test.rs:4:17: 4:29
StorageDead(_3); // scope 1 at $DIR/derefer_test.rs:4:28: 4:29
StorageLive(_4); // scope 2 at $DIR/derefer_test.rs:5:9: 5:10
- _4 = &mut ((*(_2.1: &mut (i32, i32))).0: i32); // scope 2 at $DIR/derefer_test.rs:5:13: 5:26
+ StorageLive(_6); // scope 2 at $DIR/derefer_test.rs:5:13: 5:26
+ _6 = move (_2.1: &mut (i32, i32)); // scope 2 at $DIR/derefer_test.rs:5:13: 5:26
+ _4 = &mut ((*_6).0: i32); // scope 2 at $DIR/derefer_test.rs:5:13: 5:26
+ _4 = &mut ((*_6).0: i32); // scope 2 at $DIR/derefer_test.rs:5:13: 5:26
StorageLive(_5); // scope 3 at $DIR/derefer_test.rs:6:9: 6:10
- _5 = &mut ((*(_2.1: &mut (i32, i32))).1: i32); // scope 3 at $DIR/derefer_test.rs:6:13: 6:26
+ StorageLive(_7); // scope 3 at $DIR/derefer_test.rs:6:13: 6:26
+ _7 = move (_2.1: &mut (i32, i32)); // scope 3 at $DIR/derefer_test.rs:6:13: 6:26
+ _5 = &mut ((*_7).1: i32); // scope 3 at $DIR/derefer_test.rs:6:13: 6:26
+ _5 = &mut ((*_7).1: i32); // scope 3 at $DIR/derefer_test.rs:6:13: 6:26
_0 = const (); // scope 0 at $DIR/derefer_test.rs:2:11: 7:2
StorageDead(_5); // scope 3 at $DIR/derefer_test.rs:7:1: 7:2
StorageDead(_4); // scope 2 at $DIR/derefer_test.rs:7:1: 7:2
StorageDead(_2); // scope 1 at $DIR/derefer_test.rs:7:1: 7:2
StorageDead(_1); // scope 0 at $DIR/derefer_test.rs:7:1: 7:2
return; // scope 0 at $DIR/derefer_test.rs:7:2: 7:2
+ }
+
+ bb1 (cleanup): {
+ resume; // scope 0 at $DIR/derefer_test.rs:2:1: 7:2
}
}

7 changes: 7 additions & 0 deletions src/test/mir-opt/derefer_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// EMIT_MIR derefer_test.main.Derefer.diff
fn main() {
let mut a = (42,43);
let mut b = (99, &mut a);
let x = &mut (*b.1).0;
let y = &mut (*b.1).1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ fn b(_1: &mut Box<T>) -> &mut T {
debug self => _4; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _5: &mut T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _6: &mut T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _7: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
}

bb0: {
Expand All @@ -19,7 +20,10 @@ fn b(_1: &mut Box<T>) -> &mut T {
_4 = &mut (*_1); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:15
StorageLive(_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
StorageLive(_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_6 = &mut (*(*_4)); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
StorageLive(_7); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_7 = move (*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_6 = &mut (*_7); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_6 = &mut (*_7); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_5 = &mut (*_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_3 = &mut (*_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
StorageDead(_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ fn d(_1: &Box<T>) -> &T {
let mut _3: &std::boxed::Box<T>; // in scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:5: 18:15
scope 1 (inlined <Box<T> as AsRef<T>>::as_ref) { // at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:5: 18:15
debug self => _3; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _4: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:5: 18:15
StorageLive(_3); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:5: 18:15
_3 = &(*_1); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:5: 18:15
_2 = &(*(*_3)); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
StorageLive(_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_4 = move (*_3); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_2 = &(*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_2 = &(*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_0 = &(*_2); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:5: 18:15
StorageDead(_3); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:18:14: 18:15
StorageDead(_2); // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:19:1: 19:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,9 @@
StorageDead(_3); // scope 0 at $DIR/lower_array_len.rs:11:5: 11:6
return; // scope 0 at $DIR/lower_array_len.rs:12:2: 12:2
}

bb6 (cleanup): {
resume; // scope 0 at $DIR/lower_array_len.rs:6:1: 12:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,9 @@
StorageDead(_3); // scope 0 at $DIR/lower_array_len.rs:24:5: 24:6
return; // scope 0 at $DIR/lower_array_len.rs:25:2: 25:2
}

bb7 (cleanup): {
resume; // scope 0 at $DIR/lower_array_len.rs:17:1: 25:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,9 @@
StorageDead(_2); // scope 0 at $DIR/lower_array_len.rs:31:13: 31:14
return; // scope 0 at $DIR/lower_array_len.rs:32:2: 32:2
}

bb2 (cleanup): {
resume; // scope 0 at $DIR/lower_array_len.rs:30:1: 32:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,9 @@
StorageDead(_2); // scope 0 at $DIR/lower_array_len.rs:38:13: 38:14
return; // scope 0 at $DIR/lower_array_len.rs:39:2: 39:2
}

bb2 (cleanup): {
resume; // scope 0 at $DIR/lower_array_len.rs:37:1: 39:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,9 @@
StorageDead(_3); // scope 0 at $DIR/lower_slice_len.rs:9:5: 9:6
return; // scope 0 at $DIR/lower_slice_len.rs:10:2: 10:2
}

bb6 (cleanup): {
resume; // scope 0 at $DIR/lower_slice_len.rs:4:1: 10:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
bb0: {
_2 = discriminant(_1); // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:22:11: 22:12
- switchInt(move _2) -> [1_isize: bb3, 2_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:22:5: 22:12
+ switchInt(move _2) -> [1_isize: bb3, 2_isize: bb2, otherwise: bb5]; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:22:5: 22:12
+ switchInt(move _2) -> [1_isize: bb3, 2_isize: bb2, otherwise: bb6]; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:22:5: 22:12
}

bb1: {
Expand All @@ -29,9 +29,13 @@

bb4: {
return; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:27:2: 27:2
}

bb5 (cleanup): {
resume; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:21:1: 27:2
+ }
+
+ bb5: {
+ bb6: {
+ unreachable; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:25:14: 25:15
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,9 @@
bb4: {
return; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:18:2: 18:2
}

bb5 (cleanup): {
resume; // scope 0 at $DIR/uninhabited_fallthrough_elimination.rs:12:1: 18:2
}
}

4 changes: 4 additions & 0 deletions src/test/mir-opt/unreachable.main.UnreachablePropagation.diff
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
_0 = const (); // scope 0 at $DIR/unreachable.rs:19:6: 19:6
StorageDead(_1); // scope 0 at $DIR/unreachable.rs:20:1: 20:2
return; // scope 0 at $DIR/unreachable.rs:20:2: 20:2
- }
-
- bb7 (cleanup): {
- resume; // scope 0 at $DIR/unreachable.rs:8:1: 20:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@
StorageDead(_1); // scope 0 at $DIR/unreachable_diverging.rs:20:1: 20:2
StorageDead(_2); // scope 0 at $DIR/unreachable_diverging.rs:20:1: 20:2
return; // scope 0 at $DIR/unreachable_diverging.rs:20:2: 20:2
- }
-
- bb7 (cleanup): {
- resume; // scope 0 at $DIR/unreachable_diverging.rs:12:1: 20:2
}
}

4 changes: 4 additions & 0 deletions src/test/run-make/const_fn_mir/dump.mir
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ fn foo() -> i32 {
_0 = move (_1.0: i32); // scope 0 at main.rs:5:5: 5:10
return; // scope 0 at main.rs:6:2: 6:2
}

bb2 (cleanup): {
resume; // scope 0 at main.rs:4:1: 6:2
}
}

fn main() -> () {
Expand Down