Skip to content

Commit

Permalink
Auto merge of #35409 - eddyb:mir-storage-stmts, r=nikomatsakis
Browse files Browse the repository at this point in the history
[MIR] Add Storage{Live,Dead} statements to emit llvm.lifetime.{start,end}.

Storage live ranges are tracked for all MIR variables and temporaries with a drop scope.
`StorageLive` is lowered to `llvm.lifetime.start` and `StorageDead` to `llvm.lifetime.end`.

There are some improvements possible here, such as:
* pack multiple storage liveness statements by using the index of first local + `u64` bitset
* enforce that locals are not directly accessed outside their storage live range
* shrink storage live ranges for never-borrowed locals to initialization -> last use
* emit storage liveness statements for *all* temporaries
 * however, the remaining ones are *always* SSA immediates, so they'd be noop in MIR trans
 * could have a flag on the temporary that its storage is irrelevant (a la C's old `register`)
   * would also deny borrows if necessary
    * this seems like an overcompliation and with packing & optimizations it may be pointless

Even in the current state, it helps stage2 `rustc` compile `boiler` without overflowing (see #35408).

A later addition fixes #26764 and closes #27372 by emitting `.section` directives for dylib metadata to avoid them being allocated into memory or read as `.note`. For this PR, those bugs were tripping valgrind.
  • Loading branch information
bors authored Aug 14, 2016
2 parents 92ae4ce + 1bb1444 commit 1d5b758
Show file tree
Hide file tree
Showing 23 changed files with 387 additions and 85 deletions.
13 changes: 12 additions & 1 deletion src/librustc/mir/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,15 +688,26 @@ pub struct Statement<'tcx> {

#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
pub enum StatementKind<'tcx> {
/// Write the RHS Rvalue to the LHS Lvalue.
Assign(Lvalue<'tcx>, Rvalue<'tcx>),
SetDiscriminant{ lvalue: Lvalue<'tcx>, variant_index: usize },

/// Write the discriminant for a variant to the enum Lvalue.
SetDiscriminant { lvalue: Lvalue<'tcx>, variant_index: usize },

/// Start a live range for the storage of the local.
StorageLive(Lvalue<'tcx>),

/// End the current live range for the storage of the local.
StorageDead(Lvalue<'tcx>),
}

impl<'tcx> Debug for Statement<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
use self::StatementKind::*;
match self.kind {
Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv),
StorageLive(ref lv) => write!(fmt, "StorageLive({:?})", lv),
StorageDead(ref lv) => write!(fmt, "StorageDead({:?})", lv),
SetDiscriminant{lvalue: ref lv, variant_index: index} => {
write!(fmt, "discriminant({:?}) = {:?}", lv, index)
}
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ macro_rules! make_mir_visitor {
StatementKind::SetDiscriminant{ ref $($mutability)* lvalue, .. } => {
self.visit_lvalue(lvalue, LvalueContext::Store);
}
StatementKind::StorageLive(ref $($mutability)* lvalue) => {
self.visit_lvalue(lvalue, LvalueContext::StorageLive);
}
StatementKind::StorageDead(ref $($mutability)* lvalue) => {
self.visit_lvalue(lvalue, LvalueContext::StorageDead);
}
}
}

Expand Down Expand Up @@ -759,4 +765,8 @@ pub enum LvalueContext {

// Consumed as part of an operand
Consume,

// Starting and ending a storage live range
StorageLive,
StorageDead,
}
2 changes: 2 additions & 0 deletions src/librustc_borrowck/borrowck/mir/dataflow/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,8 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
sets.kill_set.add(&moi);
});
}
repr::StatementKind::StorageLive(_) |
repr::StatementKind::StorageDead(_) => {}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
repr::StatementKind::Assign(ref lvalue, ref rvalue) => {
(lvalue, rvalue)
}
repr::StatementKind::StorageLive(_) |
repr::StatementKind::StorageDead(_) => continue,
repr::StatementKind::SetDiscriminant{ .. } =>
span_bug!(stmt.source_info.span,
"sanity_check should run before Deaggregator inserts SetDiscriminant"),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_borrowck/borrowck/mir/gather_moves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,8 @@ fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> MoveD
Rvalue::InlineAsm { .. } => {}
}
}
StatementKind::StorageLive(_) |
StatementKind::StorageDead(_) => {}
StatementKind::SetDiscriminant{ .. } => {
span_bug!(stmt.source_info.span,
"SetDiscriminant should not exist during borrowck");
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_borrowck/borrowck/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,8 @@ fn drop_flag_effects_for_location<'a, 'tcx, F>(
move_data.rev_lookup.find(lvalue),
|moi| callback(moi, DropFlagState::Present))
}
repr::StatementKind::StorageLive(_) |
repr::StatementKind::StorageDead(_) => {}
},
None => {
debug!("drop_flag_effects: replace {:?}", block.terminator());
Expand Down
41 changes: 18 additions & 23 deletions src/librustc_metadata/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,34 +867,29 @@ fn get_metadata_section_imp(target: &Target, flavor: CrateFlavor, filename: &Pat
}

pub fn meta_section_name(target: &Target) -> &'static str {
// Historical note:
//
// When using link.exe it was seen that the section name `.note.rustc`
// was getting shortened to `.note.ru`, and according to the PE and COFF
// specification:
//
// > Executable images do not use a string table and do not support
// > section names longer than 8 characters
//
// https://msdn.microsoft.com/en-us/library/windows/hardware/gg463119.aspx
//
// As a result, we choose a slightly shorter name! As to why
// `.note.rustc` works on MinGW, that's another good question...

if target.options.is_like_osx {
"__DATA,__note.rustc"
} else if target.options.is_like_msvc {
// When using link.exe it was seen that the section name `.note.rustc`
// was getting shortened to `.note.ru`, and according to the PE and COFF
// specification:
//
// > Executable images do not use a string table and do not support
// > section names longer than 8 characters
//
// https://msdn.microsoft.com/en-us/library/windows/hardware/gg463119.aspx
//
// As a result, we choose a slightly shorter name! As to why
// `.note.rustc` works on MinGW, that's another good question...
".rustc"
"__DATA,.rustc"
} else {
".note.rustc"
".rustc"
}
}

pub fn read_meta_section_name(target: &Target) -> &'static str {
if target.options.is_like_osx {
"__note.rustc"
} else if target.options.is_like_msvc {
".rustc"
} else {
".note.rustc"
}
pub fn read_meta_section_name(_target: &Target) -> &'static str {
".rustc"
}

// A diagnostic function for dumping crate metadata to an output stream
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_mir/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// FIXME #30046 ^~~~
this.expr_into_pattern(block, pattern, init)
}));
} else {
this.storage_live_for_bindings(block, &pattern);
}

// Enter the visibility scope, after evaluating the initializer.
Expand Down
9 changes: 8 additions & 1 deletion src/librustc_mir/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let temp = this.temp(expr_ty.clone());
let temp_lifetime = expr.temp_lifetime;
let expr_span = expr.span;
let source_info = this.source_info(expr_span);

if temp_lifetime.is_some() {
this.cfg.push(block, Statement {
source_info: source_info,
kind: StatementKind::StorageLive(temp.clone())
});
}

// Careful here not to cause an infinite cycle. If we always
// called `into`, then for lvalues like `x.f`, it would
Expand All @@ -49,7 +57,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
Category::Lvalue => {
let lvalue = unpack!(block = this.as_lvalue(block, expr));
let rvalue = Rvalue::Use(Operand::Consume(lvalue));
let source_info = this.source_info(expr_span);
this.cfg.push_assign(block, source_info, &temp, rvalue);
}
_ => {
Expand Down
42 changes: 42 additions & 0 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
PatternKind::Binding { mode: BindingMode::ByValue,
var,
subpattern: None, .. } => {
self.storage_live_for_bindings(block, &irrefutable_pat);
let lvalue = Lvalue::Var(self.var_indices[&var]);
return self.into(&lvalue, block, initializer);
}
Expand Down Expand Up @@ -206,6 +207,43 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
var_scope
}

/// Emit `StorageLive` for every binding in the pattern.
pub fn storage_live_for_bindings(&mut self,
block: BasicBlock,
pattern: &Pattern<'tcx>) {
match *pattern.kind {
PatternKind::Binding { var, ref subpattern, .. } => {
let lvalue = Lvalue::Var(self.var_indices[&var]);
let source_info = self.source_info(pattern.span);
self.cfg.push(block, Statement {
source_info: source_info,
kind: StatementKind::StorageLive(lvalue)
});

if let Some(subpattern) = subpattern.as_ref() {
self.storage_live_for_bindings(block, subpattern);
}
}
PatternKind::Array { ref prefix, ref slice, ref suffix } |
PatternKind::Slice { ref prefix, ref slice, ref suffix } => {
for subpattern in prefix.iter().chain(slice).chain(suffix) {
self.storage_live_for_bindings(block, subpattern);
}
}
PatternKind::Constant { .. } | PatternKind::Range { .. } | PatternKind::Wild => {
}
PatternKind::Deref { ref subpattern } => {
self.storage_live_for_bindings(block, subpattern);
}
PatternKind::Leaf { ref subpatterns } |
PatternKind::Variant { ref subpatterns, .. } => {
for subpattern in subpatterns {
self.storage_live_for_bindings(block, &subpattern.pattern);
}
}
}
}
}

/// List of blocks for each arm (and potentially other metadata in the
Expand Down Expand Up @@ -665,6 +703,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
};

let source_info = self.source_info(binding.span);
self.cfg.push(block, Statement {
source_info: source_info,
kind: StatementKind::StorageLive(Lvalue::Var(var_index))
});
self.cfg.push_assign(block, source_info,
&Lvalue::Var(var_index), rvalue);
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ pub fn construct_const<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>,
let span = tcx.map.span(item_id);
let mut builder = Builder::new(hir, span);

let extent = ROOT_CODE_EXTENT;
let extent = tcx.region_maps.temporary_scope(ast_expr.id)
.unwrap_or(ROOT_CODE_EXTENT);
let mut block = START_BLOCK;
let _ = builder.in_scope(extent, block, |builder| {
let expr = builder.hir.mirror(ast_expr);
Expand Down
Loading

0 comments on commit 1d5b758

Please sign in to comment.