Skip to content

Commit dce1c61

Browse files
committed
Add dropflag hints (stack-local booleans) for unfragmented paths in trans.
Added code to maintain these hints at runtime, and to conditionalize drop-filling and calls to destructors. In this early stage, we are using hints, so we are always free to leave out a flag for a path -- then we just pass `None` as the dropflag hint in the corresponding schedule cleanup call. But, once a path has a hint, we must at least maintain it: i.e. if the hint exists, we must ensure it is never set to "moved" if the data in question might actually have been initialized. It remains sound to conservatively set the hint to "initialized" as long as the true drop-flag embedded in the value itself is up-to-date. ---- Here are some high-level details I want to point out: * We maintain the hint in Lvalue::post_store, marking the lvalue as moved. (But also continue drop-filling if necessary.) * We update the hint on ExprAssign. * We pass along the hint in once closures that capture-by-move. * You only call `drop_ty` for state that does not have an associated hint. If you have a hint, you must call `drop_ty_core` instead. (Originally I passed the hint into `drop_ty` as well, to make the connection to a hint more apparent, but the vast majority of current calls to `drop_ty` are in contexts where no hint is available, so it just seemed like noise in the resulting diff.)
1 parent d3d552b commit dce1c61

File tree

10 files changed

+259
-70
lines changed

10 files changed

+259
-70
lines changed

src/librustc_trans/trans/_match.rs

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ use trans::build::{AddCase, And, Br, CondBr, GEPi, InBoundsGEP, Load, PointerCas
205205
use trans::build::{Not, Store, Sub, add_comment};
206206
use trans::build;
207207
use trans::callee;
208-
use trans::cleanup::{self, CleanupMethods};
208+
use trans::cleanup::{self, CleanupMethods, DropHintMethods};
209209
use trans::common::*;
210210
use trans::consts;
211211
use trans::datum::*;
@@ -947,14 +947,14 @@ fn insert_lllocals<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
947947
TrByCopy(llbinding) |
948948
TrByMoveIntoCopy(llbinding) => {
949949
let llval = Load(bcx, binding_info.llmatch);
950-
let lval = match binding_info.trmode {
950+
let lvalue = match binding_info.trmode {
951951
TrByCopy(..) =>
952952
Lvalue::new("_match::insert_lllocals"),
953953
TrByMoveIntoCopy(..) =>
954954
Lvalue::match_input("_match::insert_lllocals", bcx, binding_info.id),
955955
_ => unreachable!(),
956956
};
957-
let datum = Datum::new(llval, binding_info.ty, lval);
957+
let datum = Datum::new(llval, binding_info.ty, lvalue);
958958
call_lifetime_start(bcx, llbinding);
959959
bcx = datum.store_to(bcx, llbinding);
960960
if let Some(cs) = cs {
@@ -971,14 +971,15 @@ fn insert_lllocals<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
971971
TrByRef => (binding_info.llmatch, true),
972972
};
973973

974-
let lval = Lvalue::local("_match::insert_lllocals",
975-
bcx,
976-
binding_info.id,
977-
aliases_other_state);
978-
let datum = Datum::new(llval, binding_info.ty, lval);
974+
let lvalue = Lvalue::local("_match::insert_lllocals",
975+
bcx,
976+
binding_info.id,
977+
aliases_other_state);
978+
let datum = Datum::new(llval, binding_info.ty, lvalue);
979979
if let Some(cs) = cs {
980+
let opt_datum = lvalue.dropflag_hint(bcx);
980981
bcx.fcx.schedule_lifetime_end(cs, binding_info.llmatch);
981-
bcx.fcx.schedule_drop_and_fill_mem(cs, llval, binding_info.ty);
982+
bcx.fcx.schedule_drop_and_fill_mem(cs, llval, binding_info.ty, opt_datum);
982983
}
983984

984985
debug!("binding {} to {}", binding_info.id, bcx.val_to_string(llval));
@@ -1505,13 +1506,13 @@ fn create_bindings_map<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, pat: &ast::Pat,
15051506
// but during matching we need to store a *T as explained
15061507
// above
15071508
llmatch = alloca_no_lifetime(bcx,
1508-
llvariable_ty.ptr_to(),
1509-
&bcx.name(name));
1509+
llvariable_ty.ptr_to(),
1510+
&bcx.name(name));
15101511
trmode = TrByMoveRef;
15111512
}
15121513
ast::BindByRef(_) => {
15131514
llmatch = alloca_no_lifetime(bcx,
1514-
llvariable_ty,
1515+
llvariable_ty,
15151516
&bcx.name(name));
15161517
trmode = TrByRef;
15171518
}
@@ -1631,7 +1632,25 @@ pub fn store_local<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
16311632
bcx = mk_binding_alloca(
16321633
bcx, p_id, path1.node.name, scope, (),
16331634
"_match::store_local::create_dummy_locals",
1634-
|(), bcx, llval, ty| { drop_done_fill_mem(bcx, llval, ty); bcx });
1635+
|(), bcx, Datum { val: llval, ty, kind }| {
1636+
// Dummy-locals start out uninitialized, so set their
1637+
// drop-flag hints (if any) to "moved."
1638+
if let Some(hint) = kind.dropflag_hint(bcx) {
1639+
let moved_hint = adt::DTOR_MOVED_HINT as usize;
1640+
debug!("store moved_hint={} for hint={:?}, uninitialized dummy",
1641+
moved_hint, hint);
1642+
Store(bcx, C_u8(bcx.fcx.ccx, moved_hint), hint.to_value().value());
1643+
}
1644+
1645+
if kind.drop_flag_info.must_zero() {
1646+
// if no drop-flag hint, or the hint requires
1647+
// we maintain the embedded drop-flag, then
1648+
// mark embedded drop-flag(s) as moved
1649+
// (i.e. "already dropped").
1650+
drop_done_fill_mem(bcx, llval, ty);
1651+
}
1652+
bcx
1653+
});
16351654
});
16361655
bcx
16371656
}
@@ -1654,8 +1673,8 @@ pub fn store_local<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
16541673
return mk_binding_alloca(
16551674
bcx, pat.id, ident.name, var_scope, (),
16561675
"_match::store_local",
1657-
|(), bcx, v, _| expr::trans_into(bcx, &**init_expr,
1658-
expr::SaveIn(v)));
1676+
|(), bcx, Datum { val: v, .. }| expr::trans_into(bcx, &**init_expr,
1677+
expr::SaveIn(v)));
16591678
}
16601679

16611680
None => {}
@@ -1684,23 +1703,23 @@ fn mk_binding_alloca<'blk, 'tcx, A, F>(bcx: Block<'blk, 'tcx>,
16841703
caller_name: &'static str,
16851704
populate: F)
16861705
-> Block<'blk, 'tcx> where
1687-
F: FnOnce(A, Block<'blk, 'tcx>, ValueRef, Ty<'tcx>) -> Block<'blk, 'tcx>,
1706+
F: FnOnce(A, Block<'blk, 'tcx>, Datum<'tcx, Lvalue>) -> Block<'blk, 'tcx>,
16881707
{
16891708
let var_ty = node_id_type(bcx, p_id);
16901709

16911710
// Allocate memory on stack for the binding.
16921711
let llval = alloc_ty(bcx, var_ty, &bcx.name(name));
1712+
let lvalue = Lvalue::binding(caller_name, bcx, p_id, name);
1713+
let datum = Datum::new(llval, var_ty, lvalue);
16931714

16941715
// Subtle: be sure that we *populate* the memory *before*
16951716
// we schedule the cleanup.
1696-
let bcx = populate(arg, bcx, llval, var_ty);
1717+
let bcx = populate(arg, bcx, datum);
16971718
bcx.fcx.schedule_lifetime_end(cleanup_scope, llval);
1698-
bcx.fcx.schedule_drop_mem(cleanup_scope, llval, var_ty);
1719+
bcx.fcx.schedule_drop_mem(cleanup_scope, llval, var_ty, lvalue.dropflag_hint(bcx));
16991720

17001721
// Now that memory is initialized and has cleanup scheduled,
1701-
// create the datum and insert into the local variable map.
1702-
let lval = Lvalue::binding(caller_name, bcx, p_id, name);
1703-
let datum = Datum::new(llval, var_ty, lval);
1722+
// insert datum into the local variable map.
17041723
bcx.fcx.lllocals.borrow_mut().insert(p_id, datum);
17051724
bcx
17061725
}
@@ -1746,7 +1765,7 @@ pub fn bind_irrefutable_pat<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
17461765
bcx = mk_binding_alloca(
17471766
bcx, pat.id, path1.node.name, cleanup_scope, (),
17481767
"_match::bind_irrefutable_pat",
1749-
|(), bcx, llval, ty| {
1768+
|(), bcx, Datum { val: llval, ty, kind: _ }| {
17501769
match pat_binding_mode {
17511770
ast::BindByValue(_) => {
17521771
// By value binding: move the value that `val`
@@ -1854,10 +1873,7 @@ pub fn bind_irrefutable_pat<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
18541873
ast::PatBox(ref inner) => {
18551874
let llbox = Load(bcx, val.val);
18561875
bcx = bind_irrefutable_pat(
1857-
bcx,
1858-
&**inner,
1859-
MatchInput::from_val(llbox),
1860-
cleanup_scope);
1876+
bcx, &**inner, MatchInput::from_val(llbox), cleanup_scope);
18611877
}
18621878
ast::PatRegion(ref inner, _) => {
18631879
let loaded_val = Load(bcx, val.val);
@@ -1884,13 +1900,13 @@ pub fn bind_irrefutable_pat<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
18841900
.chain(slice.iter())
18851901
.chain(after.iter())
18861902
.zip(extracted.vals)
1887-
.fold(bcx, |bcx, (inner, elem)|
1903+
.fold(bcx, |bcx, (inner, elem)| {
18881904
bind_irrefutable_pat(
18891905
bcx,
18901906
&**inner,
18911907
MatchInput::from_val(elem),
18921908
cleanup_scope)
1893-
);
1909+
});
18941910
}
18951911
ast::PatMac(..) => {
18961912
bcx.sess().span_bug(pat.span, "unexpanded macro");

src/librustc_trans/trans/adt.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,20 @@ macro_rules! repeat_u8_as_u64 {
163163
(repeat_u8_as_u32!($name) as u64)) }
164164
}
165165

166+
/// `DTOR_NEEDED_HINT` is a stack-local hint that just means
167+
/// "we do not know whether the destructor has run or not; check the
168+
/// drop-flag embedded in the value itself."
169+
pub const DTOR_NEEDED_HINT: u8 = 0x3d;
170+
171+
/// `DTOR_MOVED_HINT` is a stack-local hint that means "this value has
172+
/// definitely been moved; you do not need to run its destructor."
173+
///
174+
/// (However, for now, such values may still end up being explicitly
175+
/// zeroed by the generated code; this is the distinction between
176+
/// `datum::DropFlagInfo::ZeroAndMaintain` versus
177+
/// `datum::DropFlagInfo::DontZeroJustUse`.)
178+
pub const DTOR_MOVED_HINT: u8 = 0x2d;
179+
166180
pub const DTOR_NEEDED: u8 = 0xd4;
167181
pub const DTOR_NEEDED_U32: u32 = repeat_u8_as_u32!(DTOR_NEEDED);
168182
pub const DTOR_NEEDED_U64: u64 = repeat_u8_as_u64!(DTOR_NEEDED);

src/librustc_trans/trans/base.rs

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ use trans::attributes;
5151
use trans::build::*;
5252
use trans::builder::{Builder, noname};
5353
use trans::callee;
54-
use trans::cleanup::CleanupMethods;
55-
use trans::cleanup;
54+
use trans::cleanup::{self, CleanupMethods, DropHint};
5655
use trans::closure;
5756
use trans::common::{Block, C_bool, C_bytes_in_context, C_i32, C_int, C_integral};
5857
use trans::common::{C_null, C_struct_in_context, C_u64, C_u8, C_undef};
@@ -88,7 +87,7 @@ use arena::TypedArena;
8887
use libc::c_uint;
8988
use std::ffi::{CStr, CString};
9089
use std::cell::{Cell, RefCell};
91-
use std::collections::HashSet;
90+
use std::collections::{HashMap, HashSet};
9291
use std::mem;
9392
use std::str;
9493
use std::{i8, i16, i32, i64};
@@ -1284,6 +1283,54 @@ pub fn init_function<'a, 'tcx>(fcx: &'a FunctionContext<'a, 'tcx>,
12841283
}
12851284
}
12861285

1286+
// Create the drop-flag hints for every unfragmented path in the function.
1287+
let tcx = fcx.ccx.tcx();
1288+
let fn_did = ast::DefId { krate: ast::LOCAL_CRATE, node: fcx.id };
1289+
let mut hints = fcx.lldropflag_hints.borrow_mut();
1290+
let fragment_infos = tcx.fragment_infos.borrow();
1291+
1292+
// Intern table for drop-flag hint datums.
1293+
let mut seen = HashMap::new();
1294+
1295+
if let Some(fragment_infos) = fragment_infos.get(&fn_did) {
1296+
for &info in fragment_infos {
1297+
1298+
let make_datum = |id| {
1299+
let init_val = C_u8(fcx.ccx, adt::DTOR_NEEDED_HINT as usize);
1300+
let llname = &format!("dropflag_hint_{}", id);
1301+
debug!("adding hint {}", llname);
1302+
let ptr = alloc_ty(entry_bcx, tcx.types.u8, llname);
1303+
Store(entry_bcx, init_val, ptr);
1304+
let ty = tcx.mk_ptr(ty::TypeAndMut { ty: tcx.types.u8, mutbl: ast::MutMutable });
1305+
let flag = datum::Lvalue::new_dropflag_hint("base::init_function");
1306+
let datum = datum::Datum::new(ptr, ty, flag);
1307+
datum
1308+
};
1309+
1310+
let (var, datum) = match info {
1311+
ty::FragmentInfo::Moved { var, .. } |
1312+
ty::FragmentInfo::Assigned { var, .. } => {
1313+
let datum = seen.get(&var).cloned().unwrap_or_else(|| {
1314+
let datum = make_datum(var);
1315+
seen.insert(var, datum.clone());
1316+
datum
1317+
});
1318+
(var, datum)
1319+
}
1320+
};
1321+
match info {
1322+
ty::FragmentInfo::Moved { move_expr: expr_id, .. } => {
1323+
debug!("FragmentInfo::Moved insert drop hint for {}", expr_id);
1324+
hints.insert(expr_id, DropHint::new(var, datum));
1325+
}
1326+
ty::FragmentInfo::Assigned { assignee_id: expr_id, .. } => {
1327+
debug!("FragmentInfo::Assigned insert drop hint for {}", expr_id);
1328+
hints.insert(expr_id, DropHint::new(var, datum));
1329+
}
1330+
}
1331+
}
1332+
}
1333+
12871334
entry_bcx
12881335
}
12891336

src/librustc_trans/trans/cleanup.rs

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,23 @@ pub struct DropHint<K>(pub ast::NodeId, pub K);
219219
pub type DropHintDatum<'tcx> = DropHint<Datum<'tcx, Lvalue>>;
220220
pub type DropHintValue = DropHint<ValueRef>;
221221

222+
impl<K> DropHint<K> {
223+
pub fn new(id: ast::NodeId, k: K) -> DropHint<K> { DropHint(id, k) }
224+
}
225+
226+
impl DropHint<ValueRef> {
227+
pub fn value(&self) -> ValueRef { self.1 }
228+
}
229+
230+
pub trait DropHintMethods {
231+
type ValueKind;
232+
fn to_value(&self) -> Self::ValueKind;
233+
}
234+
impl<'tcx> DropHintMethods for DropHintDatum<'tcx> {
235+
type ValueKind = DropHintValue;
236+
fn to_value(&self) -> DropHintValue { DropHint(self.0, self.1.val) }
237+
}
238+
222239
impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
223240
/// Invoked when we start to trans the code contained within a new cleanup scope.
224241
fn push_ast_cleanup_scope(&self, debug_loc: NodeIdAndSpan) {
@@ -389,14 +406,17 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
389406
fn schedule_drop_mem(&self,
390407
cleanup_scope: ScopeId,
391408
val: ValueRef,
392-
ty: Ty<'tcx>) {
409+
ty: Ty<'tcx>,
410+
drop_hint: Option<DropHintDatum<'tcx>>) {
393411
if !self.type_needs_drop(ty) { return; }
412+
let drop_hint = drop_hint.map(|hint|hint.to_value());
394413
let drop = box DropValue {
395414
is_immediate: false,
396415
val: val,
397416
ty: ty,
398417
fill_on_drop: false,
399418
skip_dtor: false,
419+
drop_hint: drop_hint,
400420
};
401421

402422
debug!("schedule_drop_mem({:?}, val={}, ty={:?}) fill_on_drop={} skip_dtor={}",
@@ -413,23 +433,28 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
413433
fn schedule_drop_and_fill_mem(&self,
414434
cleanup_scope: ScopeId,
415435
val: ValueRef,
416-
ty: Ty<'tcx>) {
436+
ty: Ty<'tcx>,
437+
drop_hint: Option<DropHintDatum<'tcx>>) {
417438
if !self.type_needs_drop(ty) { return; }
418439

440+
let drop_hint = drop_hint.map(|datum|datum.to_value());
419441
let drop = box DropValue {
420442
is_immediate: false,
421443
val: val,
422444
ty: ty,
423445
fill_on_drop: true,
424446
skip_dtor: false,
447+
drop_hint: drop_hint,
425448
};
426449

427-
debug!("schedule_drop_and_fill_mem({:?}, val={}, ty={:?}, fill_on_drop={}, skip_dtor={})",
450+
debug!("schedule_drop_and_fill_mem({:?}, val={}, ty={:?},
451+
fill_on_drop={}, skip_dtor={}, has_drop_hint={})",
428452
cleanup_scope,
429453
self.ccx.tn().val_to_string(val),
430454
ty,
431455
drop.fill_on_drop,
432-
drop.skip_dtor);
456+
drop.skip_dtor,
457+
drop_hint.is_some());
433458

434459
self.schedule_clean(cleanup_scope, drop as CleanupObj);
435460
}
@@ -453,6 +478,7 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
453478
ty: ty,
454479
fill_on_drop: false,
455480
skip_dtor: true,
481+
drop_hint: None,
456482
};
457483

458484
debug!("schedule_drop_adt_contents({:?}, val={}, ty={:?}) fill_on_drop={} skip_dtor={}",
@@ -472,13 +498,14 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
472498
ty: Ty<'tcx>) {
473499

474500
if !self.type_needs_drop(ty) { return; }
475-
let drop = box DropValue {
501+
let drop = Box::new(DropValue {
476502
is_immediate: true,
477503
val: val,
478504
ty: ty,
479505
fill_on_drop: false,
480506
skip_dtor: false,
481-
};
507+
drop_hint: None,
508+
});
482509

483510
debug!("schedule_drop_immediate({:?}, val={}, ty={:?}) fill_on_drop={} skip_dtor={}",
484511
cleanup_scope,
@@ -983,6 +1010,7 @@ pub struct DropValue<'tcx> {
9831010
ty: Ty<'tcx>,
9841011
fill_on_drop: bool,
9851012
skip_dtor: bool,
1013+
drop_hint: Option<DropHintValue>,
9861014
}
9871015

9881016
impl<'tcx> Cleanup<'tcx> for DropValue<'tcx> {
@@ -1007,7 +1035,7 @@ impl<'tcx> Cleanup<'tcx> for DropValue<'tcx> {
10071035
let bcx = if self.is_immediate {
10081036
glue::drop_ty_immediate(bcx, self.val, self.ty, debug_loc, self.skip_dtor)
10091037
} else {
1010-
glue::drop_ty_core(bcx, self.val, self.ty, debug_loc, self.skip_dtor)
1038+
glue::drop_ty_core(bcx, self.val, self.ty, debug_loc, self.skip_dtor, self.drop_hint)
10111039
};
10121040
if self.fill_on_drop {
10131041
base::drop_done_fill_mem(bcx, self.val, self.ty);
@@ -1135,11 +1163,13 @@ pub trait CleanupMethods<'blk, 'tcx> {
11351163
fn schedule_drop_mem(&self,
11361164
cleanup_scope: ScopeId,
11371165
val: ValueRef,
1138-
ty: Ty<'tcx>);
1166+
ty: Ty<'tcx>,
1167+
drop_hint: Option<DropHintDatum<'tcx>>);
11391168
fn schedule_drop_and_fill_mem(&self,
11401169
cleanup_scope: ScopeId,
11411170
val: ValueRef,
1142-
ty: Ty<'tcx>);
1171+
ty: Ty<'tcx>,
1172+
drop_hint: Option<DropHintDatum<'tcx>>);
11431173
fn schedule_drop_adt_contents(&self,
11441174
cleanup_scope: ScopeId,
11451175
val: ValueRef,

0 commit comments

Comments
 (0)