From cf0e78bd3bcfd1010f8933acc6e134410016f6f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Mon, 13 Mar 2023 00:00:00 +0000 Subject: [PATCH] Use index based drop loop for slices and arrays Instead of building two kinds of drop pair loops, of which only one will be eventually used at runtime in a given monomorphization, always use index based loop. --- .../rustc_mir_dataflow/src/elaborate_drops.rs | 133 +++++------------- ...[String].AddMovesForPackedDrops.before.mir | 80 +++-------- 2 files changed, 49 insertions(+), 164 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs index bd12087629c..486275570bd 100644 --- a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs +++ b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs @@ -655,26 +655,20 @@ where /// /// ```text /// loop-block: - /// can_go = cur == length_or_end + /// can_go = cur == len /// if can_go then succ else drop-block /// drop-block: - /// if ptr_based { - /// ptr = cur - /// cur = cur.offset(1) - /// } else { - /// ptr = &raw mut P[cur] - /// cur = cur + 1 - /// } + /// ptr = &raw mut P[cur] + /// cur = cur + 1 /// drop(ptr) /// ``` fn drop_loop( &mut self, succ: BasicBlock, cur: Local, - length_or_end: Place<'tcx>, + len: Local, ety: Ty<'tcx>, unwind: Unwind, - ptr_based: bool, ) -> BasicBlock { let copy = |place: Place<'tcx>| Operand::Copy(place); let move_ = |place: Place<'tcx>| Operand::Move(place); @@ -683,22 +677,19 @@ where let ptr_ty = tcx.mk_ptr(ty::TypeAndMut { ty: ety, mutbl: hir::Mutability::Mut }); let ptr = Place::from(self.new_temp(ptr_ty)); let can_go = Place::from(self.new_temp(tcx.types.bool)); - let one = self.constant_usize(1); - let (ptr_next, cur_next) = if ptr_based { - ( - Rvalue::Use(copy(cur.into())), - Rvalue::BinaryOp(BinOp::Offset, Box::new((move_(cur.into()), one))), - ) - } else { - ( - Rvalue::AddressOf(Mutability::Mut, tcx.mk_place_index(self.place, cur)), - Rvalue::BinaryOp(BinOp::Add, Box::new((move_(cur.into()), one))), - ) - }; let drop_block = BasicBlockData { - statements: vec![self.assign(ptr, ptr_next), self.assign(Place::from(cur), cur_next)], + statements: vec![ + self.assign( + ptr, + Rvalue::AddressOf(Mutability::Mut, tcx.mk_place_index(self.place, cur)), + ), + self.assign( + cur.into(), + Rvalue::BinaryOp(BinOp::Add, Box::new((move_(cur.into()), one))), + ), + ], is_cleanup: unwind.is_cleanup(), terminator: Some(Terminator { source_info: self.source_info, @@ -711,10 +702,7 @@ where let loop_block = BasicBlockData { statements: vec![self.assign( can_go, - Rvalue::BinaryOp( - BinOp::Eq, - Box::new((copy(Place::from(cur)), copy(length_or_end))), - ), + Rvalue::BinaryOp(BinOp::Eq, Box::new((copy(Place::from(cur)), copy(len.into())))), )], is_cleanup: unwind.is_cleanup(), terminator: Some(Terminator { @@ -738,13 +726,6 @@ where fn open_drop_for_array(&mut self, ety: Ty<'tcx>, opt_size: Option) -> BasicBlock { debug!("open_drop_for_array({:?}, {:?})", ety, opt_size); - - // if size_of::() == 0 { - // index_based_loop - // } else { - // ptr_based_loop - // } - let tcx = self.tcx(); if let Some(size) = opt_size { @@ -770,86 +751,36 @@ where } } - let move_ = |place: Place<'tcx>| Operand::Move(place); - let elem_size = Place::from(self.new_temp(tcx.types.usize)); - let len = Place::from(self.new_temp(tcx.types.usize)); - - let base_block = BasicBlockData { - statements: vec![ - self.assign(elem_size, Rvalue::NullaryOp(NullOp::SizeOf, ety)), - self.assign(len, Rvalue::Len(self.place)), - ], - is_cleanup: self.unwind.is_cleanup(), - terminator: Some(Terminator { - source_info: self.source_info, - kind: TerminatorKind::SwitchInt { - discr: move_(elem_size), - targets: SwitchTargets::static_if( - 0, - self.drop_loop_pair(ety, false, len), - self.drop_loop_pair(ety, true, len), - ), - }, - }), - }; - self.elaborator.patch().new_block(base_block) + self.drop_loop_pair(ety) } /// Creates a pair of drop-loops of `place`, which drops its contents, even - /// in the case of 1 panic. If `ptr_based`, creates a pointer loop, - /// otherwise create an index loop. - fn drop_loop_pair( - &mut self, - ety: Ty<'tcx>, - ptr_based: bool, - length: Place<'tcx>, - ) -> BasicBlock { - debug!("drop_loop_pair({:?}, {:?})", ety, ptr_based); + /// in the case of 1 panic. + fn drop_loop_pair(&mut self, ety: Ty<'tcx>) -> BasicBlock { + debug!("drop_loop_pair({:?})", ety); let tcx = self.tcx(); - let iter_ty = if ptr_based { tcx.mk_mut_ptr(ety) } else { tcx.types.usize }; + let len = self.new_temp(tcx.types.usize); + let cur = self.new_temp(tcx.types.usize); - let cur = self.new_temp(iter_ty); - let length_or_end = if ptr_based { Place::from(self.new_temp(iter_ty)) } else { length }; + let unwind = + self.unwind.map(|unwind| self.drop_loop(unwind, cur, len, ety, Unwind::InCleanup)); - let unwind = self.unwind.map(|unwind| { - self.drop_loop(unwind, cur, length_or_end, ety, Unwind::InCleanup, ptr_based) - }); + let loop_block = self.drop_loop(self.succ, cur, len, ety, unwind); - let loop_block = self.drop_loop(self.succ, cur, length_or_end, ety, unwind, ptr_based); - - let cur = Place::from(cur); - let drop_block_stmts = if ptr_based { - let tmp_ty = tcx.mk_mut_ptr(self.place_ty(self.place)); - let tmp = Place::from(self.new_temp(tmp_ty)); - // tmp = &raw mut P; - // cur = tmp as *mut T; - // end = Offset(cur, len); - let mir_cast_kind = ty::cast::mir_cast_kind(iter_ty, tmp_ty); - vec![ - self.assign(tmp, Rvalue::AddressOf(Mutability::Mut, self.place)), - self.assign(cur, Rvalue::Cast(mir_cast_kind, Operand::Move(tmp), iter_ty)), - self.assign( - length_or_end, - Rvalue::BinaryOp( - BinOp::Offset, - Box::new((Operand::Copy(cur), Operand::Move(length))), - ), - ), - ] - } else { - // cur = 0 (length already pushed) - let zero = self.constant_usize(0); - vec![self.assign(cur, Rvalue::Use(zero))] - }; - let drop_block = self.elaborator.patch().new_block(BasicBlockData { - statements: drop_block_stmts, + let zero = self.constant_usize(0); + let block = BasicBlockData { + statements: vec![ + self.assign(len.into(), Rvalue::Len(self.place)), + self.assign(cur.into(), Rvalue::Use(zero)), + ], is_cleanup: unwind.is_cleanup(), terminator: Some(Terminator { source_info: self.source_info, kind: TerminatorKind::Goto { target: loop_block }, }), - }); + }; + let drop_block = self.elaborator.patch().new_block(block); // FIXME(#34708): handle partially-dropped array/slice elements. let reset_block = self.drop_flag_reset_block(DropFlagMode::Deep, drop_block, unwind); self.drop_flag_test_block(reset_block, self.succ, unwind) diff --git a/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String].AddMovesForPackedDrops.before.mir b/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String].AddMovesForPackedDrops.before.mir index 391b00effac..3884d29db41 100644 --- a/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String].AddMovesForPackedDrops.before.mir +++ b/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String].AddMovesForPackedDrops.before.mir @@ -4,21 +4,13 @@ fn std::ptr::drop_in_place(_1: *mut [String]) -> () { let mut _0: (); // return place in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 let mut _2: usize; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 let mut _3: usize; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _4: usize; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _5: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _6: bool; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _7: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _8: bool; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _9: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _10: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _11: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _12: bool; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _13: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _14: bool; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _15: *mut [std::string::String]; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + let mut _4: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + let mut _5: bool; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + let mut _6: *mut std::string::String; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + let mut _7: bool; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 bb0: { - goto -> bb15; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + goto -> bb8; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } bb1: { @@ -30,72 +22,34 @@ fn std::ptr::drop_in_place(_1: *mut [String]) -> () { } bb3 (cleanup): { - _5 = &raw mut (*_1)[_4]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _4 = Add(move _4, const 1_usize); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - drop((*_5)) -> bb4; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _4 = &raw mut (*_1)[_3]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _3 = Add(move _3, const 1_usize); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + drop((*_4)) -> bb4; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } bb4 (cleanup): { - _6 = Eq(_4, _3); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - switchInt(move _6) -> [0: bb3, otherwise: bb2]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _5 = Eq(_3, _2); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + switchInt(move _5) -> [0: bb3, otherwise: bb2]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } bb5: { - _7 = &raw mut (*_1)[_4]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _4 = Add(move _4, const 1_usize); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - drop((*_7)) -> [return: bb6, unwind: bb4]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _6 = &raw mut (*_1)[_3]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _3 = Add(move _3, const 1_usize); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + drop((*_6)) -> [return: bb6, unwind: bb4]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } bb6: { - _8 = Eq(_4, _3); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - switchInt(move _8) -> [0: bb5, otherwise: bb1]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _7 = Eq(_3, _2); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + switchInt(move _7) -> [0: bb5, otherwise: bb1]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } bb7: { - _4 = const 0_usize; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _2 = Len((*_1)); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _3 = const 0_usize; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 goto -> bb6; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } bb8: { goto -> bb7; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 } - - bb9 (cleanup): { - _11 = _9; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _9 = Offset(move _9, const 1_usize); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - drop((*_11)) -> bb10; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } - - bb10 (cleanup): { - _12 = Eq(_9, _10); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - switchInt(move _12) -> [0: bb9, otherwise: bb2]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } - - bb11: { - _13 = _9; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _9 = Offset(move _9, const 1_usize); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - drop((*_13)) -> [return: bb12, unwind: bb10]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } - - bb12: { - _14 = Eq(_9, _10); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - switchInt(move _14) -> [0: bb11, otherwise: bb1]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } - - bb13: { - _15 = &raw mut (*_1); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _9 = move _15 as *mut std::string::String (PtrToPtr); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _10 = Offset(_9, move _3); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - goto -> bb12; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } - - bb14: { - goto -> bb13; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } - - bb15: { - _2 = SizeOf(std::string::String); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _3 = Len((*_1)); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - switchInt(move _2) -> [0: bb8, otherwise: bb14]; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - } }