Skip to content

Commit

Permalink
Auto merge of rust-lang#134297 - scottmcm:drop-arrays-by-unsizing, r=…
Browse files Browse the repository at this point in the history
…<try>

Stop emitting drop loops for every array length

We can just unsize the array to a slice and drop that instead, reusing the same slice loop for every array length.

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`.

Demonstration that yes, every slice length gets its own loop today: <https://rust.godbolt.org/z/5EsPjPWv4>
  • Loading branch information
bors committed Dec 14, 2024
2 parents 4a204be + f6cd47a commit 41701b3
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 63 deletions.
124 changes: 62 additions & 62 deletions compiler/rustc_mir_dataflow/src/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -738,70 +739,52 @@ where
loop_block
}

fn open_drop_for_array(&mut self, ety: Ty<'tcx>, opt_size: Option<u64>) -> 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<u64>,
) -> BasicBlock {
debug!("open_drop_for_array({:?}, {:?})", element_ty, opt_size);
let tcx = self.tcx();

if let Some(size) = opt_size {
enum ProjectionKind<Path> {
Drop(std::ops::Range<u64>),
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::<Vec<_>>();
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
Expand All @@ -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(),
Expand Down Expand Up @@ -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),

Expand Down
Original file line number Diff line number Diff line change
@@ -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];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions tests/mir-opt/slice_drop_shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_);
}

0 comments on commit 41701b3

Please sign in to comment.