From f6cd47a0b1665dd0b3772829eb1721365af7f1fd Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Fri, 13 Dec 2024 20:14:48 -0800 Subject: [PATCH] Stop emitting drop loops for every array length We can just unsize the array to a slice and drop that instead, reusing the same loop. As part of this (and how I originally noticed this), use `PtrMetadata` instead of `Len` for the slice length, for another step closer to being able to remove `Rvalue::Len`. --- .../rustc_mir_dataflow/src/elaborate_drops.rs | 124 +++++++++--------- ...ring;42].AddMovesForPackedDrops.before.mir | 25 ++++ ...[String].AddMovesForPackedDrops.before.mir | 2 +- tests/mir-opt/slice_drop_shim.rs | 2 + 4 files changed, 90 insertions(+), 63 deletions(-) create mode 100644 tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String;42].AddMovesForPackedDrops.before.mir diff --git a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs index 494b7d54d8a12..17a81f309898a 100644 --- a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs +++ b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs @@ -6,6 +6,7 @@ use rustc_index::Idx; use rustc_middle::mir::patch::MirPatch; use rustc_middle::mir::*; use rustc_middle::span_bug; +use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::util::IntTypeExt; use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt}; use rustc_span::DUMMY_SP; @@ -738,70 +739,52 @@ where loop_block } - fn open_drop_for_array(&mut self, ety: Ty<'tcx>, opt_size: Option) -> BasicBlock { - debug!("open_drop_for_array({:?}, {:?})", ety, opt_size); + fn open_drop_for_array( + &mut self, + place_ty: Ty<'tcx>, + element_ty: Ty<'tcx>, + opt_size: Option, + ) -> BasicBlock { + debug!("open_drop_for_array({:?}, {:?})", element_ty, opt_size); let tcx = self.tcx(); - if let Some(size) = opt_size { - enum ProjectionKind { - Drop(std::ops::Range), - Keep(u64, Path), - } - // Previously, we'd make a projection for every element in the array and create a drop - // ladder if any `array_subpath` was `Some`, i.e. moving out with an array pattern. - // This caused huge memory usage when generating the drops for large arrays, so we instead - // record the *subslices* which are dropped and the *indexes* which are kept - let mut drop_ranges = vec![]; - let mut dropping = true; - let mut start = 0; - for i in 0..size { - let path = self.elaborator.array_subpath(self.path, i, size); - if dropping && path.is_some() { - drop_ranges.push(ProjectionKind::Drop(start..i)); - dropping = false; - } else if !dropping && path.is_none() { - dropping = true; - start = i; - } - if let Some(path) = path { - drop_ranges.push(ProjectionKind::Keep(i, path)); - } - } - if !drop_ranges.is_empty() { - if dropping { - drop_ranges.push(ProjectionKind::Drop(start..size)); - } - let fields = drop_ranges - .iter() - .rev() - .map(|p| { - let (project, path) = match p { - ProjectionKind::Drop(r) => ( - ProjectionElem::Subslice { - from: r.start, - to: r.end, - from_end: false, - }, - None, - ), - &ProjectionKind::Keep(offset, path) => ( - ProjectionElem::ConstantIndex { - offset, - min_length: size, - from_end: false, - }, - Some(path), - ), - }; - (tcx.mk_place_elem(self.place, project), path) - }) - .collect::>(); - let (succ, unwind) = self.drop_ladder_bottom(); - return self.drop_ladder(fields, succ, unwind).0; - } + if let Some(0) = opt_size { + span_bug!(self.source_info.span, "Opened drop for zero-length array of {element_ty:?}") } - self.drop_loop_pair(ety) + let array_ptr_ty = Ty::new_mut_ptr(tcx, place_ty); + let array_ptr = self.new_temp(array_ptr_ty); + let slice_ty = Ty::new_slice(tcx, element_ty); + let slice_ptr_ty = Ty::new_mut_ptr(tcx, slice_ty); + let slice_ptr = self.new_temp(slice_ptr_ty); + + let unsize_and_drop_block = BasicBlockData { + statements: vec![ + self.assign(Place::from(array_ptr), Rvalue::RawPtr(Mutability::Mut, self.place)), + self.assign( + Place::from(slice_ptr), + Rvalue::Cast( + CastKind::PointerCoercion( + PointerCoercion::Unsize, + CoercionSource::Implicit, + ), + Operand::Move(Place::from(array_ptr)), + slice_ptr_ty, + ), + ), + ], + is_cleanup: self.unwind.is_cleanup(), + terminator: Some(Terminator { + source_info: self.source_info, + kind: TerminatorKind::Drop { + place: Place::from(slice_ptr).project_deeper(&[PlaceElem::Deref], tcx), + target: self.succ, + unwind: self.unwind.into_action(), + replace: false, + }, + }), + }; + self.elaborator.patch().new_block(unsize_and_drop_block) } /// Creates a pair of drop-loops of `place`, which drops its contents, even @@ -817,10 +800,27 @@ where let loop_block = self.drop_loop(self.succ, cur, len, ety, unwind); + let place_ty = self.place_ty(self.place); + assert!(place_ty.is_slice(), "Expected slice, got {place_ty:?}"); + + let [PlaceElem::Deref] = self.place.projection.as_slice() else { + span_bug!( + self.source_info.span, + "Expected place for slice drop shim to be *_n, but it's {:?}", + self.place, + ); + }; + let zero = self.constant_usize(0); let block = BasicBlockData { statements: vec![ - self.assign(len.into(), Rvalue::Len(self.place)), + self.assign( + len.into(), + Rvalue::UnaryOp( + UnOp::PtrMetadata, + Operand::Copy(Place::from(self.place.local)), + ), + ), self.assign(cur.into(), Rvalue::Use(zero)), ], is_cleanup: unwind.is_cleanup(), @@ -863,7 +863,7 @@ where ty::Dynamic(..) => self.complete_drop(self.succ, self.unwind), ty::Array(ety, size) => { let size = size.try_to_target_usize(self.tcx()); - self.open_drop_for_array(*ety, size) + self.open_drop_for_array(ty, *ety, size) } ty::Slice(ety) => self.drop_loop_pair(*ety), diff --git a/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String;42].AddMovesForPackedDrops.before.mir b/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String;42].AddMovesForPackedDrops.before.mir new file mode 100644 index 0000000000000..84b4c12fdec2b --- /dev/null +++ b/tests/mir-opt/slice_drop_shim.core.ptr-drop_in_place.[String;42].AddMovesForPackedDrops.before.mir @@ -0,0 +1,25 @@ +// MIR for `std::ptr::drop_in_place` before AddMovesForPackedDrops + +fn std::ptr::drop_in_place(_1: *mut [String; 42]) -> () { + let mut _0: (); + let mut _2: *mut [std::string::String; 42]; + let mut _3: *mut [std::string::String]; + + bb0: { + goto -> bb3; + } + + bb1: { + return; + } + + bb2 (cleanup): { + resume; + } + + bb3: { + _2 = &raw mut (*_1); + _3 = move _2 as *mut [std::string::String] (PointerCoercion(Unsize, Implicit)); + drop((*_3)) -> [return: bb1, unwind: bb2]; + } +} 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 4d1eaa6ffe324..0633b765644d5 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 @@ -44,7 +44,7 @@ fn std::ptr::drop_in_place(_1: *mut [String]) -> () { } bb7: { - _2 = Len((*_1)); + _2 = PtrMetadata(copy _1); _3 = const 0_usize; goto -> bb6; } diff --git a/tests/mir-opt/slice_drop_shim.rs b/tests/mir-opt/slice_drop_shim.rs index c2f4c82ecc864..f34c34855a169 100644 --- a/tests/mir-opt/slice_drop_shim.rs +++ b/tests/mir-opt/slice_drop_shim.rs @@ -5,6 +5,8 @@ // if we use -Clink-dead-code. // EMIT_MIR core.ptr-drop_in_place.[String].AddMovesForPackedDrops.before.mir +// EMIT_MIR core.ptr-drop_in_place.[String;42].AddMovesForPackedDrops.before.mir fn main() { let _fn = std::ptr::drop_in_place::<[String]> as unsafe fn(_); + let _fn = std::ptr::drop_in_place::<[String; 42]> as unsafe fn(_); }