Skip to content

Commit 0f76dc7

Browse files
committed
Auto merge of #157491 - cjgillot:elaborate-drop-chain, r=<try>
Shrink no-op drop elaboration
2 parents 39ec825 + c7f866c commit 0f76dc7

18 files changed

Lines changed: 1671 additions & 323 deletions

compiler/rustc_mir_transform/src/elaborate_drop.rs

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::{fmt, iter, mem};
22

3+
use itertools::Itertools;
34
use rustc_abi::{FIRST_VARIANT, FieldIdx, VariantIdx};
45
use rustc_hir::lang_items::LangItem;
56
use rustc_hir::{CoroutineDesugaring, CoroutineKind};
@@ -798,6 +799,7 @@ where
798799

799800
(tcx.mk_place_field(base_place, field_idx, field_ty), subpath)
800801
})
802+
.filter(|path| self.should_retain_for_ladder(path))
801803
.collect()
802804
}
803805

@@ -875,6 +877,19 @@ where
875877
)
876878
}
877879

880+
/// Whether this drop is useful. This is purely an optimization to avoid generating useless blocks.
881+
fn should_retain_for_ladder(&self, (place, subpath): &(Place<'tcx>, Option<D::Path>)) -> bool {
882+
if !self.place_ty(*place).needs_drop(self.tcx(), self.elaborator.typing_env()) {
883+
return false;
884+
}
885+
if let Some(subpath) = subpath
886+
&& let DropStyle::Dead = self.elaborator.drop_style(*subpath, DropFlagMode::Deep)
887+
{
888+
return false;
889+
}
890+
true
891+
}
892+
878893
/// Creates a full drop ladder, consisting of 2 connected half-drop-ladders
879894
///
880895
/// For example, with 3 fields, the drop ladder is
@@ -915,7 +930,7 @@ where
915930
#[instrument(level = "debug", skip(self), ret)]
916931
fn drop_ladder(
917932
&mut self,
918-
fields: Vec<(Place<'tcx>, Option<D::Path>)>,
933+
mut fields: Vec<(Place<'tcx>, Option<D::Path>)>,
919934
succ: BasicBlock,
920935
unwind: Unwind,
921936
dropline: Option<BasicBlock>,
@@ -925,10 +940,7 @@ where
925940
"Dropline is set for cleanup drop ladder"
926941
);
927942

928-
let mut fields = fields;
929-
fields.retain(|&(place, _)| {
930-
self.place_ty(place).needs_drop(self.tcx(), self.elaborator.typing_env())
931-
});
943+
fields.retain(|path| self.should_retain_for_ladder(path));
932944

933945
debug!("drop_ladder - fields needing drop: {:?}", fields);
934946

@@ -1149,15 +1161,21 @@ where
11491161
if !have_otherwise {
11501162
values.pop();
11511163
} else if !have_otherwise_with_drop_glue {
1152-
normal_blocks.push(self.goto_block(succ, unwind));
1164+
normal_blocks.push(succ);
11531165
if let Unwind::To(unwind) = unwind {
1154-
unwind_blocks.push(self.goto_block(unwind, Unwind::InCleanup));
1166+
unwind_blocks.push(unwind);
1167+
}
1168+
if let Some(dropline) = dropline {
1169+
dropline_blocks.push(dropline);
11551170
}
11561171
} else {
11571172
normal_blocks.push(self.drop_block(succ, unwind));
11581173
if let Unwind::To(unwind) = unwind {
11591174
unwind_blocks.push(self.drop_block(unwind, Unwind::InCleanup));
11601175
}
1176+
if let Some(dropline) = dropline {
1177+
dropline_blocks.push(self.drop_block(dropline, unwind));
1178+
}
11611179
}
11621180

11631181
(
@@ -1179,27 +1197,29 @@ where
11791197
succ: BasicBlock,
11801198
unwind: Unwind,
11811199
) -> BasicBlock {
1182-
// If there are multiple variants, then if something
1183-
// is present within the enum the discriminant, tracked
1184-
// by the rest path, must be initialized.
1185-
//
1186-
// Additionally, we do not want to switch on the
1187-
// discriminant after it is free-ed, because that
1188-
// way lies only trouble.
1189-
let discr_ty = adt.repr().discr_type().to_ty(self.tcx());
1190-
let discr = Place::from(self.new_temp(discr_ty));
1191-
let discr_rv = Rvalue::Discriminant(self.place);
1192-
let switch_block = self.new_block_with_statements(
1193-
unwind,
1194-
vec![self.assign(discr, discr_rv)],
1195-
TerminatorKind::SwitchInt {
1196-
discr: Operand::Move(discr),
1197-
targets: SwitchTargets::new(
1198-
values.iter().copied().zip(blocks.iter().copied()),
1199-
*blocks.last().unwrap(),
1200-
),
1201-
},
1202-
);
1200+
let switch_block = blocks.iter().copied().all_equal_value().unwrap_or_else(|_| {
1201+
// If there are multiple variants, then if something
1202+
// is present within the enum the discriminant, tracked
1203+
// by the rest path, must be initialized.
1204+
//
1205+
// Additionally, we do not want to switch on the
1206+
// discriminant after it is free-ed, because that
1207+
// way lies only trouble.
1208+
let discr_ty = adt.repr().discr_type().to_ty(self.tcx());
1209+
let discr = Place::from(self.new_temp(discr_ty));
1210+
let discr_rv = Rvalue::Discriminant(self.place);
1211+
self.new_block_with_statements(
1212+
unwind,
1213+
vec![self.assign(discr, discr_rv)],
1214+
TerminatorKind::SwitchInt {
1215+
discr: Operand::Move(discr),
1216+
targets: SwitchTargets::new(
1217+
values.iter().copied().zip(blocks.iter().copied()),
1218+
*blocks.last().unwrap(),
1219+
),
1220+
},
1221+
)
1222+
});
12031223
self.drop_flag_test_block(switch_block, succ, unwind)
12041224
}
12051225

@@ -1580,11 +1600,6 @@ where
15801600
}
15811601
}
15821602

1583-
fn goto_block(&mut self, target: BasicBlock, unwind: Unwind) -> BasicBlock {
1584-
let block = TerminatorKind::Goto { target };
1585-
self.new_block(unwind, block)
1586-
}
1587-
15881603
/// Returns the block to jump to in order to test the drop flag and execute the drop.
15891604
///
15901605
/// Depending on the required `DropStyle`, this might be a generated block with an `if`

0 commit comments

Comments
 (0)