Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MIR] Add Storage{Live,Dead} statements to emit llvm.lifetime.{start,end}. #35409

Merged
merged 2 commits into from
Aug 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make a helper function in cfg, since you’re constructing this multiple times manually.

This should become this.cfg.push_storage_live(source_info, temp);

});
}

// 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a bug -- but was your discovery of it prompted by some test case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing constant expression testcases started failing. The 1+2 temporary in [T; 1+2+3] has a scope that's the function's. You think I should dig deeper into middle::region and make it the same as if it was outside of a function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing constant expression testcases started failing. The 1+2 temporary in [T; 1+2+3] has a scope that's the function's. You think I should dig deeper into middle::region and make it the same as if it was outside of a functions?

No, just trying to understand where this change came from; it seemed kind of out of the blue. I'm a bit confused still though. What is [T; 1+2+3]? Some expression in a functon?

.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