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.

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

Move Derefer before Retag #96549

Merged
merged 3 commits into from
May 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,8 @@ pub enum LocalInfo<'tcx> {
/// A temporary created during the creation of an aggregate
/// (e.g. a temporary for `foo` in `MyStruct { my_field: foo }`)
AggregateTemp,
/// A temporary created during the pass `Derefer` to avoid it's retagging
Copy link
Member

Choose a reason for hiding this comment

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

(should be "its retagging"... not a big deal though)

Temp,
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}

impl<'tcx> LocalDecl<'tcx> {
Expand Down Expand Up @@ -1178,6 +1180,10 @@ impl<'tcx> LocalDecl<'tcx> {
}
}

pub fn temp(mut self) {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
self.local_info = Some(Box::new(LocalInfo::Temp));
}

/// Converts `self` into same `LocalDecl` except tagged as internal.
#[inline]
pub fn internal(mut self) -> Self {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/mir/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ impl<'tcx> MirPatch<'tcx> {
Location { block: bb, statement_index: offset }
}

pub fn new_local_temp(&mut self, ty: Ty<'tcx>, span: Span) -> Local {
ouz-a marked this conversation as resolved.
Show resolved Hide resolved
let index = self.next_local;
self.next_local += 1;
let mut new_decl = LocalDecl::new(ty, span);
new_decl.local_info = Some(Box::new(LocalInfo::Temp));
self.new_locals.push(new_decl);
Local::new(index as usize)
}

pub fn new_temp(&mut self, ty: Ty<'tcx>, span: Span) -> Local {
let index = self.next_local;
self.next_local += 1;
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_mir_transform/src/add_retag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ fn may_be_reference(ty: Ty<'_>) -> bool {
}
}

/// Determines whether or not this LocalDecl is temp, if not it needs retagging.
fn is_not_temp<'tcx>(local_decl: &LocalDecl<'tcx>) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

is_temp would be slightly clearer I think; return false inside this function is a double negation which is somewhat confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I will change this function in the next update of derefer.

if local_decl.local_info.is_some() {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
match local_decl.local_info.as_ref().unwrap().as_ref() {
LocalInfo::Temp => return false,
_ => (),
};
}
return true;
}

impl<'tcx> MirPass<'tcx> for AddRetag {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.opts.debugging_opts.mir_emit_retag
Expand All @@ -71,7 +82,9 @@ impl<'tcx> MirPass<'tcx> for AddRetag {
let needs_retag = |place: &Place<'tcx>| {
// FIXME: Instead of giving up for unstable places, we should introduce
// a temporary and retag on that.
is_stable(place.as_ref()) && may_be_reference(place.ty(&*local_decls, tcx).ty)
is_stable(place.as_ref())
&& may_be_reference(place.ty(&*local_decls, tcx).ty)
&& is_not_temp(&local_decls[place.local])
};
let place_base_raw = |place: &Place<'tcx>| {
// If this is a `Deref`, get the type of what we are deref'ing.
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/deref_separator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ impl<'tcx> MutVisitor<'tcx> for DerefChecker<'tcx> {
if p_elem == ProjectionElem::Deref && !p_ref.projection.is_empty() {
let ty = p_ref.ty(&self.local_decls, self.tcx).ty;
let temp =
self.patcher.new_temp(ty, self.local_decls[p_ref.local].source_info.span);
self.patcher.new_local_temp(ty, self.local_decls[p_ref.local].source_info.span);

self.patcher.add_statement(loc, StatementKind::StorageLive(temp));

// We are adding current p_ref's projections to our
// temp value, excluding projections we already covered.
let deref_place = Place::from(place_local)
.project_deeper(&p_ref.projection[last_len..], self.tcx);

self.patcher.add_assign(
loc,
Place::from(temp),
Rvalue::Use(Operand::Move(deref_place)),
);

place_local = temp;
last_len = p_ref.projection.len();

Expand All @@ -58,7 +58,7 @@ impl<'tcx> MutVisitor<'tcx> for DerefChecker<'tcx> {
*place = temp_place;
}

// We are destroying last temp since it's no longer used.
// We are destroying the previous temp since it's no longer used.
if let Some(prev_temp) = prev_temp {
self.patcher.add_statement(loc, StatementKind::StorageDead(prev_temp));
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,13 +426,13 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
&add_moves_for_packed_drops::AddMovesForPackedDrops,
// `AddRetag` needs to run after `ElaborateDrops`. Otherwise it should run fairly late,
// but before optimizations begin.
&deref_separator::Derefer,
&add_retag::AddRetag,
&lower_intrinsics::LowerIntrinsics,
&simplify::SimplifyCfg::new("elaborate-drops"),
// `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
8 changes: 4 additions & 4 deletions src/test/mir-opt/derefer_complex_case.main.Derefer.diff
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@
StorageDead(_6); // scope 1 at $DIR/derefer_complex_case.rs:4:39: 4:40
_5 = const (); // scope 1 at $DIR/derefer_complex_case.rs:4:5: 4:40
goto -> bb2; // scope 1 at $DIR/derefer_complex_case.rs:4:5: 4:40
+ }
+
+ bb8 (cleanup): {
+ resume; // scope 0 at $DIR/derefer_complex_case.rs:3:1: 5:2
}
bb8 (cleanup): {
resume; // scope 0 at $DIR/derefer_complex_case.rs:3:1: 5:2
}
}

8 changes: 4 additions & 4 deletions src/test/mir-opt/derefer_terminator_test.main.Derefer.diff
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@
StorageDead(_2); // scope 1 at $DIR/derefer_terminator_test.rs:10:1: 10:2
StorageDead(_1); // scope 0 at $DIR/derefer_terminator_test.rs:10:1: 10:2
return; // scope 0 at $DIR/derefer_terminator_test.rs:10:2: 10:2
+ }
+
+ bb6 (cleanup): {
+ resume; // scope 0 at $DIR/derefer_terminator_test.rs:2:1: 10:2
}
bb6 (cleanup): {
resume; // scope 0 at $DIR/derefer_terminator_test.rs:2:1: 10:2
}
}

16 changes: 6 additions & 10 deletions src/test/mir-opt/derefer_test.main.Derefer.diff
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,11 @@

bb0: {
StorageLive(_1); // scope 0 at $DIR/derefer_test.rs:3:9: 3:14
Deinit(_1); // scope 0 at $DIR/derefer_test.rs:3:17: 3:24
(_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
_1 = (const 42_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
Deinit(_2); // scope 1 at $DIR/derefer_test.rs:4:17: 4:29
(_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
_2 = (const 99_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
Expand All @@ -53,10 +49,10 @@
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
}
bb1 (cleanup): {
resume; // scope 0 at $DIR/derefer_test.rs:2:1: 7:2
}
}

24 changes: 8 additions & 16 deletions src/test/mir-opt/derefer_test_multiple.main.Derefer.diff
Original file line number Diff line number Diff line change
Expand Up @@ -39,29 +39,21 @@

bb0: {
StorageLive(_1); // scope 0 at $DIR/derefer_test_multiple.rs:3:9: 3:14
Deinit(_1); // scope 0 at $DIR/derefer_test_multiple.rs:3:17: 3:25
(_1.0: i32) = const 42_i32; // scope 0 at $DIR/derefer_test_multiple.rs:3:17: 3:25
(_1.1: i32) = const 43_i32; // scope 0 at $DIR/derefer_test_multiple.rs:3:17: 3:25
_1 = (const 42_i32, const 43_i32); // scope 0 at $DIR/derefer_test_multiple.rs:3:17: 3:25
StorageLive(_2); // scope 1 at $DIR/derefer_test_multiple.rs:4:9: 4:14
StorageLive(_3); // scope 1 at $DIR/derefer_test_multiple.rs:4:22: 4:28
_3 = &mut _1; // scope 1 at $DIR/derefer_test_multiple.rs:4:22: 4:28
Deinit(_2); // scope 1 at $DIR/derefer_test_multiple.rs:4:17: 4:29
(_2.0: i32) = const 99_i32; // scope 1 at $DIR/derefer_test_multiple.rs:4:17: 4:29
(_2.1: &mut (i32, i32)) = move _3; // scope 1 at $DIR/derefer_test_multiple.rs:4:17: 4:29
_2 = (const 99_i32, move _3); // scope 1 at $DIR/derefer_test_multiple.rs:4:17: 4:29
StorageDead(_3); // scope 1 at $DIR/derefer_test_multiple.rs:4:28: 4:29
StorageLive(_4); // scope 2 at $DIR/derefer_test_multiple.rs:5:9: 5:14
StorageLive(_5); // scope 2 at $DIR/derefer_test_multiple.rs:5:22: 5:28
_5 = &mut _2; // scope 2 at $DIR/derefer_test_multiple.rs:5:22: 5:28
Deinit(_4); // scope 2 at $DIR/derefer_test_multiple.rs:5:17: 5:29
(_4.0: i32) = const 11_i32; // scope 2 at $DIR/derefer_test_multiple.rs:5:17: 5:29
(_4.1: &mut (i32, &mut (i32, i32))) = move _5; // scope 2 at $DIR/derefer_test_multiple.rs:5:17: 5:29
_4 = (const 11_i32, move _5); // scope 2 at $DIR/derefer_test_multiple.rs:5:17: 5:29
StorageDead(_5); // scope 2 at $DIR/derefer_test_multiple.rs:5:28: 5:29
StorageLive(_6); // scope 3 at $DIR/derefer_test_multiple.rs:6:9: 6:14
StorageLive(_7); // scope 3 at $DIR/derefer_test_multiple.rs:6:22: 6:28
_7 = &mut _4; // scope 3 at $DIR/derefer_test_multiple.rs:6:22: 6:28
Deinit(_6); // scope 3 at $DIR/derefer_test_multiple.rs:6:17: 6:29
(_6.0: i32) = const 13_i32; // scope 3 at $DIR/derefer_test_multiple.rs:6:17: 6:29
(_6.1: &mut (i32, &mut (i32, &mut (i32, i32)))) = move _7; // scope 3 at $DIR/derefer_test_multiple.rs:6:17: 6:29
_6 = (const 13_i32, move _7); // scope 3 at $DIR/derefer_test_multiple.rs:6:17: 6:29
StorageDead(_7); // scope 3 at $DIR/derefer_test_multiple.rs:6:28: 6:29
StorageLive(_8); // scope 4 at $DIR/derefer_test_multiple.rs:7:9: 7:10
- _8 = &mut ((*((*((*(_6.1: &mut (i32, &mut (i32, &mut (i32, i32))))).1: &mut (i32, &mut (i32, i32)))).1: &mut (i32, i32))).1: i32); // scope 4 at $DIR/derefer_test_multiple.rs:7:13: 7:30
Expand Down Expand Up @@ -95,10 +87,10 @@
StorageDead(_2); // scope 1 at $DIR/derefer_test_multiple.rs:9:1: 9:2
StorageDead(_1); // scope 0 at $DIR/derefer_test_multiple.rs:9:1: 9:2
return; // scope 0 at $DIR/derefer_test_multiple.rs:9:2: 9:2
+ }
+
+ bb1 (cleanup): {
+ resume; // scope 0 at $DIR/derefer_test_multiple.rs:2:1: 9:2
}
bb1 (cleanup): {
resume; // scope 0 at $DIR/derefer_test_multiple.rs:2:1: 9:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@
+ bb3: {
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:8:1: 8:2
return; // scope 0 at $DIR/early_otherwise_branch.rs:8:2: 8:2
}

- bb5 (cleanup): {
- resume; // scope 0 at $DIR/early_otherwise_branch.rs:3:1: 8:2
+ }
+
+ bb4: {
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:4:5: 4:17
+ switchInt(_7) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:4:5: 4:17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,8 @@
+ bb4: {
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:17:1: 17:2
return; // scope 0 at $DIR/early_otherwise_branch.rs:17:2: 17:2
}

- bb7 (cleanup): {
- resume; // scope 0 at $DIR/early_otherwise_branch.rs:11:1: 17:2
+ }
+
+ bb5: {
+ StorageDead(_12); // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 12:17
+ switchInt(_8) -> [0_isize: bb3, 1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 12:17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@
+ bb3: {
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:26:1: 26:2
return; // scope 0 at $DIR/early_otherwise_branch.rs:26:2: 26:2
}

- bb5 (cleanup): {
- resume; // scope 0 at $DIR/early_otherwise_branch.rs:21:1: 26:2
+ }
+
+ bb4: {
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:22:5: 22:17
+ switchInt(_7) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:22:5: 22:17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@
+ bb4: {
StorageDead(_4); // scope 0 at $DIR/early_otherwise_branch_3_element_tuple.rs:9:1: 9:2
return; // scope 0 at $DIR/early_otherwise_branch_3_element_tuple.rs:9:2: 9:2
}

- bb6 (cleanup): {
- resume; // scope 0 at $DIR/early_otherwise_branch_3_element_tuple.rs:4:1: 9:2
+ }
+
+ bb5: {
+ StorageDead(_15); // scope 0 at $DIR/early_otherwise_branch_3_element_tuple.rs:5:5: 5:20
+ switchInt(_10) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch_3_element_tuple.rs:5:5: 5:20
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,5 @@
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch_noopt.rs:14:1: 14:2
return; // scope 0 at $DIR/early_otherwise_branch_noopt.rs:14:2: 14:2
}

bb9 (cleanup): {
resume; // scope 0 at $DIR/early_otherwise_branch_noopt.rs:7:1: 14:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,5 @@
bb5: {
return; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:27:2: 27:2
}

bb6 (cleanup): {
resume; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:18:1: 27:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,5 @@
bb4: {
return; // scope 0 at $DIR/early_otherwise_branch_soundness.rs:14:2: 14:2
}

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

Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,5 @@
StorageDead(_2); // scope 0 at $DIR/if-condition-int.rs:17:25: 17:26
return; // scope 0 at $DIR/if-condition-int.rs:18:2: 18:2
}

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:16:1: 18:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,5 @@
StorageDead(_2); // scope 0 at $DIR/if-condition-int.rs:53:34: 53:35
return; // scope 0 at $DIR/if-condition-int.rs:54:2: 54:2
}

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:52:1: 54:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,5 @@
StorageDead(_2); // scope 0 at $DIR/if-condition-int.rs:49:1: 49:2
return; // scope 0 at $DIR/if-condition-int.rs:49:2: 49:2
}

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:43:1: 49:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,5 @@
StorageDead(_2); // scope 0 at $DIR/if-condition-int.rs:21:32: 21:33
return; // scope 0 at $DIR/if-condition-int.rs:22:2: 22:2
}

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:20:1: 22:2
}
}

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

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:24:1: 26:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,5 @@
StorageDead(_2); // scope 0 at $DIR/if-condition-int.rs:39:5: 39:6
return; // scope 0 at $DIR/if-condition-int.rs:40:2: 40:2
}

bb7 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:32:1: 40:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,5 @@
StorageDead(_2); // scope 0 at $DIR/if-condition-int.rs:29:32: 29:33
return; // scope 0 at $DIR/if-condition-int.rs:30:2: 30:2
}

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:28:1: 30:2
}
}

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

bb4 (cleanup): {
resume; // scope 0 at $DIR/if-condition-int.rs:11:1: 13:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,5 @@
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
}
}

Loading