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

librustc_mir: Implement def-use chains and trivial copy propagation on MIR. #36388

Merged
merged 2 commits into from
Sep 20, 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
66 changes: 66 additions & 0 deletions src/librustc/mir/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,32 @@ impl<'tcx> Mir<'tcx> {
self.var_decls.len() +
self.temp_decls.len() + 1
}

pub fn format_local(&self, local: Local) -> String {
let mut index = local.index();
index = match index.checked_sub(self.arg_decls.len()) {
None => return format!("{:?}", Arg::new(index)),
Some(index) => index,
};
index = match index.checked_sub(self.var_decls.len()) {
None => return format!("{:?}", Var::new(index)),
Some(index) => index,
};
index = match index.checked_sub(self.temp_decls.len()) {
None => return format!("{:?}", Temp::new(index)),
Some(index) => index,
};
debug_assert!(index == 0);
return "ReturnPointer".to_string()
}

/// Changes a statement to a nop. This is both faster than deleting instructions and avoids
/// invalidating statement indices in `Location`s.
pub fn make_statement_nop(&mut self, location: Location) {
let block = &mut self[location.block];
debug_assert!(location.statement_index < block.statements.len());
block.statements[location.statement_index].make_nop()
}
}

impl<'tcx> Index<BasicBlock> for Mir<'tcx> {
Expand Down Expand Up @@ -686,6 +712,14 @@ pub struct Statement<'tcx> {
pub kind: StatementKind<'tcx>,
}

impl<'tcx> Statement<'tcx> {
/// Changes a statement to a nop. This is both faster than deleting instructions and avoids
/// invalidating statement indices in `Location`s.
pub fn make_nop(&mut self) {
self.kind = StatementKind::Nop
}
}

#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
pub enum StatementKind<'tcx> {
/// Write the RHS Rvalue to the LHS Lvalue.
Expand All @@ -699,6 +733,9 @@ pub enum StatementKind<'tcx> {

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

/// No-op. Useful for deleting instructions without affecting statement indices.
Nop,
Copy link
Member

Choose a reason for hiding this comment

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

Potential point of disagreement (we already have at least one lazy patching mechanism which preserves indices).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is that one?

Copy link
Member

Choose a reason for hiding this comment

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

It's in src/librustc_borrowck/borrowck/mir/patch.rs (because drop elaboration in there. IMO rustc_borrowck should be replaced with a MIR version that doesn't live in a rustc_borrowck crate but that's besides the point).

It doesn't have the ability to remove statements because it wasn't needed.
I'm not entirely clear on whether that functionality could be easily added. cc @arielb1

}

impl<'tcx> Debug for Statement<'tcx> {
Expand All @@ -711,6 +748,7 @@ impl<'tcx> Debug for Statement<'tcx> {
SetDiscriminant{lvalue: ref lv, variant_index: index} => {
write!(fmt, "discriminant({:?}) = {:?}", lv, index)
}
Nop => write!(fmt, "nop"),
}
}
}
Expand Down Expand Up @@ -824,6 +862,24 @@ impl<'tcx> Lvalue<'tcx> {
elem: elem,
}))
}

pub fn from_local(mir: &Mir<'tcx>, local: Local) -> Lvalue<'tcx> {
Copy link
Member

Choose a reason for hiding this comment

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

Several people wanted this already, I'm surprised it wasn't already in.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still thing we should combine args/vars/temps into one array (locals), but I haven't gotten around to playing with that....

let mut index = local.index();
index = match index.checked_sub(mir.arg_decls.len()) {
None => return Lvalue::Arg(Arg(index as u32)),
Some(index) => index,
};
index = match index.checked_sub(mir.var_decls.len()) {
None => return Lvalue::Var(Var(index as u32)),
Some(index) => index,
};
index = match index.checked_sub(mir.temp_decls.len()) {
None => return Lvalue::Temp(Temp(index as u32)),
Some(index) => index,
};
debug_assert!(index == 0);
Lvalue::ReturnPointer
}
}

impl<'tcx> Debug for Lvalue<'tcx> {
Expand Down Expand Up @@ -1258,3 +1314,13 @@ impl fmt::Debug for Location {
write!(fmt, "{:?}[{}]", self.block, self.statement_index)
}
}

impl Location {
pub fn dominates(&self, other: &Location, dominators: &Dominators<BasicBlock>) -> bool {
if self.block == other.block {
self.statement_index <= other.statement_index
} else {
dominators.is_dominated_by(other.block, self.block)
}
}
}
109 changes: 104 additions & 5 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ macro_rules! make_mir_visitor {

fn visit_lvalue(&mut self,
lvalue: & $($mutability)* Lvalue<'tcx>,
context: LvalueContext,
context: LvalueContext<'tcx>,
location: Location) {
self.super_lvalue(lvalue, context, location);
}
Expand Down Expand Up @@ -346,6 +346,7 @@ macro_rules! make_mir_visitor {
StatementKind::StorageDead(ref $($mutability)* lvalue) => {
self.visit_lvalue(lvalue, LvalueContext::StorageDead, location);
}
StatementKind::Nop => {}
}
}

Expand Down Expand Up @@ -580,7 +581,7 @@ macro_rules! make_mir_visitor {

fn super_lvalue(&mut self,
lvalue: & $($mutability)* Lvalue<'tcx>,
context: LvalueContext,
context: LvalueContext<'tcx>,
location: Location) {
match *lvalue {
Lvalue::Var(_) |
Expand All @@ -605,7 +606,12 @@ macro_rules! make_mir_visitor {
ref $($mutability)* base,
ref $($mutability)* elem,
} = *proj;
self.visit_lvalue(base, LvalueContext::Projection, location);
let context = if context.is_mutating_use() {
LvalueContext::Projection(Mutability::Mut)
} else {
LvalueContext::Projection(Mutability::Not)
};
self.visit_lvalue(base, context, location);
self.visit_projection_elem(elem, context, location);
}

Expand Down Expand Up @@ -750,6 +756,21 @@ macro_rules! make_mir_visitor {

fn super_const_usize(&mut self, _substs: & $($mutability)* ConstUsize) {
}

// Convenience methods

fn visit_location(&mut self, mir: & $($mutability)* Mir<'tcx>, location: Location) {
let basic_block = & $($mutability)* mir[location.block];
if basic_block.statements.len() == location.statement_index {
if let Some(ref $($mutability)* terminator) = basic_block.terminator {
self.visit_terminator(location.block, terminator, location)
}
} else {
let statement = & $($mutability)*
basic_block.statements[location.statement_index];
self.visit_statement(location.block, statement, location)
}
}
}
}
}
Expand All @@ -774,8 +795,20 @@ pub enum LvalueContext<'tcx> {
// Being borrowed
Borrow { region: &'tcx Region, kind: BorrowKind },

// Used as base for another lvalue, e.g. `x` in `x.y`
Projection,
// Used as base for another lvalue, e.g. `x` in `x.y`.
//
// The `Mutability` argument specifies whether the projection is being performed in order to
// (potentially) mutate the lvalue. For example, the projection `x.y` is marked as a mutation
// in these cases:
//
// x.y = ...;
// f(&mut x.y);
//
// But not in these cases:
//
// z = x.y;
// f(&x.y);
Projection(Mutability),

// Consumed as part of an operand
Consume,
Expand All @@ -784,3 +817,69 @@ pub enum LvalueContext<'tcx> {
StorageLive,
StorageDead,
}

impl<'tcx> LvalueContext<'tcx> {
/// Returns true if this lvalue context represents a drop.
pub fn is_drop(&self) -> bool {
match *self {
LvalueContext::Drop => true,
_ => false,
}
}

/// Returns true if this lvalue context represents a storage live or storage dead marker.
pub fn is_storage_marker(&self) -> bool {
match *self {
LvalueContext::StorageLive | LvalueContext::StorageDead => true,
_ => false,
}
}

/// Returns true if this lvalue context represents a storage live marker.
pub fn is_storage_live_marker(&self) -> bool {
match *self {
LvalueContext::StorageLive => true,
_ => false,
}
}

/// Returns true if this lvalue context represents a storage dead marker.
pub fn is_storage_dead_marker(&self) -> bool {
match *self {
LvalueContext::StorageDead => true,
_ => false,
}
}

/// Returns true if this lvalue context represents a use that potentially changes the value.
pub fn is_mutating_use(&self) -> bool {
match *self {
LvalueContext::Store | LvalueContext::Call |
LvalueContext::Borrow { kind: BorrowKind::Mut, .. } |
LvalueContext::Projection(Mutability::Mut) |
LvalueContext::Drop => true,
LvalueContext::Inspect |
LvalueContext::Borrow { kind: BorrowKind::Shared, .. } |
LvalueContext::Borrow { kind: BorrowKind::Unique, .. } |
LvalueContext::Projection(Mutability::Not) | LvalueContext::Consume |
LvalueContext::StorageLive | LvalueContext::StorageDead => false,
}
}

/// Returns true if this lvalue context represents a use that does not change the value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't you directly interested in !self.is_storage_marker() && !self.is_mutation()?

pub fn is_nonmutating_use(&self) -> bool {
match *self {
LvalueContext::Inspect | LvalueContext::Borrow { kind: BorrowKind::Shared, .. } |
LvalueContext::Borrow { kind: BorrowKind::Unique, .. } |
LvalueContext::Projection(Mutability::Not) | LvalueContext::Consume => true,
LvalueContext::Borrow { kind: BorrowKind::Mut, .. } | LvalueContext::Store |
LvalueContext::Call | LvalueContext::Projection(Mutability::Mut) |
LvalueContext::Drop | LvalueContext::StorageLive | LvalueContext::StorageDead => false,
}
}

pub fn is_use(&self) -> bool {
self.is_mutating_use() || self.is_nonmutating_use()
}
}

3 changes: 2 additions & 1 deletion src/librustc_borrowck/borrowck/mir/dataflow/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
});
}
repr::StatementKind::StorageLive(_) |
repr::StatementKind::StorageDead(_) => {}
repr::StatementKind::StorageDead(_) |
repr::StatementKind::Nop => {}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
(lvalue, rvalue)
}
repr::StatementKind::StorageLive(_) |
repr::StatementKind::StorageDead(_) => continue,
repr::StatementKind::StorageDead(_) |
repr::StatementKind::Nop => continue,
repr::StatementKind::SetDiscriminant{ .. } =>
span_bug!(stmt.source_info.span,
"sanity_check should run before Deaggregator inserts SetDiscriminant"),
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/mir/gather_moves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
span_bug!(stmt.source_info.span,
"SetDiscriminant should not exist during borrowck");
}
StatementKind::Nop => {}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_borrowck/borrowck/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,8 @@ fn drop_flag_effects_for_location<'a, 'tcx, F>(
|moi| callback(moi, DropFlagState::Present))
}
repr::StatementKind::StorageLive(_) |
repr::StatementKind::StorageDead(_) => {}
repr::StatementKind::StorageDead(_) |
repr::StatementKind::Nop => {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you split the addition of Nop to its own commit?

},
None => {
debug!("drop_flag_effects: replace {:?}", block.terminator());
Expand Down
1 change: 1 addition & 0 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,7 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
// No lifetime analysis based on borrowing can be done from here on out.
passes.push_pass(box mir::transform::instcombine::InstCombine::new());
passes.push_pass(box mir::transform::deaggregator::Deaggregator);
passes.push_pass(box mir::transform::copy_prop::CopyPropagation);

passes.push_pass(box mir::transform::add_call_guards::AddCallGuards);
passes.push_pass(box mir::transform::dump_mir::Marker("PreTrans"));
Expand Down
Loading