Skip to content

[MIR] Initial implementation of inlining #36593

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

Closed
wants to merge 12 commits into from
3 changes: 1 addition & 2 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub enum DepNode<D: Clone + Debug> {
// `Krate` value gives you access to all other items. To avoid
// this fate, do not call `tcx.map.krate()`; instead, prefer
// wrappers like `tcx.visit_all_items_in_krate()`. If there is no
// suitable wrapper, you can use `tcx.dep_graph.ignore()` to gain
// suitable wrapper, you can use `tcx.dep_graph.in_ignore()` to gain
// access to the krate, but you must remember to add suitable
// edges yourself for the individual items that you read.
Krate,
Expand Down Expand Up @@ -243,4 +243,3 @@ impl<D: Clone + Debug> DepNode<D> {
/// them even in the absence of a tcx.)
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct WorkProductId(pub String);

2 changes: 1 addition & 1 deletion src/librustc/mir/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ macro_rules! newtype_index {
pub struct Mir<'tcx> {
/// List of basic blocks. References to basic block use a newtyped index type `BasicBlock`
/// that indexes into this vector.
basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
pub basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

This was made private for a reason (to prevent people from forgetting to update caches). Maybe add a fn take(self) -> ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still can't do that, for the same reason I can't use the accessor methods. I'm moving stuff out of the struct, so I wouldn't be able to move the struct into a take method. Unless the method unpacked the entire MIR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless the method unpacked the entire MIR.

Personally, I think something like fn into_components(self) -> (...) seems ok.


/// List of visibility (lexical) scopes; these are referenced by statements
/// and used (eventually) for debuginfo. Indexed by a `VisibilityScope`.
Expand Down
242 changes: 242 additions & 0 deletions src/librustc_mir/callgraph.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! MIR-based callgraph.
//!
//! This only considers direct calls
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not an issue, but it's interesting that this is an "optimistic" call graph -- i.e., it is only those things that we know will be called. Often people would assume a pessimistic approach. Maybe worth highlighting just a bit in the comment here.


use rustc::hir::def_id::DefId;
use rustc_data_structures::graph;

use rustc::mir::repr::*;
use rustc::mir::visit::*;
use rustc::mir::mir_map::MirMap;

use rustc::ty;

use rustc::util::nodemap::DefIdMap;

pub struct CallGraph {
node_map: DefIdMap<graph::NodeIndex>,
graph: graph::Graph<DefId, ()>
}

impl CallGraph {
pub fn build<'tcx>(map: &MirMap<'tcx>) -> CallGraph {
let def_ids = map.map.keys();

let mut callgraph = CallGraph {
node_map: DefIdMap(),
graph: graph::Graph::new()
};

for def_id in def_ids {
let idx = callgraph.add_node(def_id);

let mut call_visitor = CallVisitor {
caller: idx,
graph: &mut callgraph
};

let mir = map.map.get(&def_id).unwrap();
call_visitor.visit_mir(&mir);
}

callgraph
}

pub fn scc_iter<'g>(&'g self) -> SCCIterator<'g> {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think you can lifetime elide 'g here

SCCIterator::new(&self.graph)
}

pub fn def_id(&self, node: graph::NodeIndex) -> DefId {
*self.graph.node_data(node)
}

fn add_node(&mut self, id: DefId) -> graph::NodeIndex {
let graph = &mut self.graph;
*self.node_map.entry(id).or_insert_with(|| {
graph.add_node(id)
})
}
}

struct CallVisitor<'a> {
caller: graph::NodeIndex,
graph: &'a mut CallGraph
}

impl<'a, 'tcx> Visitor<'tcx> for CallVisitor<'a> {
fn visit_terminator_kind(&mut self, _block: BasicBlock,
kind: &TerminatorKind<'tcx>, _loc: Location) {
if let TerminatorKind::Call {
func: Operand::Constant(ref f)
, .. } = *kind {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this formatting is pretty hard to read... can't we get it on one line? maybe use a match? Anything but this :P

if let ty::TyFnDef(def_id, _, _) = f.ty.sty {
Copy link
Contributor

@arielb1 arielb1 Sep 26, 2016

Choose a reason for hiding this comment

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

FIXME: add statically-resolvable trait calls.

let callee = self.graph.add_node(def_id);
self.graph.graph.add_edge(self.caller, callee, ());
}
}
}
}

struct StackElement<'g> {
node: graph::NodeIndex,
lowlink: usize,
children: graph::AdjacentTargets<'g, DefId, ()>
}

pub struct SCCIterator<'g> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be factored into librustc_data_structures, and ideally the graph algorithms portion of that code. But for now just open a FIXME about it.

graph: &'g graph::Graph<DefId, ()>,
index: usize,
node_indices: Vec<Option<usize>>,
scc_stack: Vec<graph::NodeIndex>,
current_scc: Vec<graph::NodeIndex>,
visit_stack: Vec<StackElement<'g>>,
}

impl<'g> SCCIterator<'g> {
pub fn new(graph: &'g graph::Graph<DefId, ()>) -> SCCIterator<'g> {
if graph.len_nodes() == 0 {
return SCCIterator {
graph: graph,
index: 0,
node_indices: Vec::new(),
scc_stack: Vec::new(),
current_scc: Vec::new(),
visit_stack: Vec::new()
};
}

let first = graph::NodeIndex(0);

SCCIterator::with_entry(graph, first)
}

pub fn with_entry(graph: &'g graph::Graph<DefId, ()>,
entry: graph::NodeIndex) -> SCCIterator<'g> {
let mut iter = SCCIterator {
graph: graph,
index: 0,
node_indices: Vec::with_capacity(graph.len_nodes()),
scc_stack: Vec::new(),
current_scc: Vec::new(),
visit_stack: Vec::new()
};

iter.visit_one(entry);

iter
}

fn get_next(&mut self) {
self.current_scc.clear();

while !self.visit_stack.is_empty() {
self.visit_children();

let node = self.visit_stack.pop().unwrap();

if let Some(last) = self.visit_stack.last_mut() {
if last.lowlink > node.lowlink {
last.lowlink = node.lowlink;
}
}

debug!("TarjanSCC: Popped node {:?} : lowlink = {:?}; index = {:?}",
node.node, node.lowlink, self.node_index(node.node).unwrap());

if node.lowlink != self.node_index(node.node).unwrap() {
continue;
}

loop {
let n = self.scc_stack.pop().unwrap();
self.current_scc.push(n);
self.set_node_index(n, !0);
if n == node.node { return; }
}
}
}

fn visit_one(&mut self, node: graph::NodeIndex) {
self.index += 1;
let idx = self.index;
self.set_node_index(node, idx);
self.scc_stack.push(node);
self.visit_stack.push(StackElement {
node: node,
lowlink: self.index,
children: self.graph.successor_nodes(node)
});
debug!("TarjanSCC: Node {:?} : index = {:?}", node, idx);
}

fn visit_children(&mut self) {
while let Some(child) = self.visit_stack.last_mut().unwrap().children.next() {
if let Some(child_num) = self.node_index(child) {
let cur = self.visit_stack.last_mut().unwrap();
if cur.lowlink > child_num {
cur.lowlink = child_num;
}
} else {
self.visit_one(child);
}
}
}

fn node_index(&self, node: graph::NodeIndex) -> Option<usize> {
self.node_indices.get(node.node_id()).and_then(|&idx| idx)
}

fn set_node_index(&mut self, node: graph::NodeIndex, idx: usize) {
let i = node.node_id();
if i >= self.node_indices.len() {
self.node_indices.resize(i + 1, None);
}
self.node_indices[i] = Some(idx);
}
}

impl<'g> Iterator for SCCIterator<'g> {
type Item = Vec<graph::NodeIndex>;

fn next(&mut self) -> Option<Vec<graph::NodeIndex>> {
self.get_next();

if self.current_scc.is_empty() {
// Try a new root for the next SCC, if the node_indices
// map is doesn't contain all nodes, use the smallest one
// with no entry, otherwise find the first empty node.
//
// FIXME: This should probably use a set of precomputed
// roots instead
if self.node_indices.len() < self.graph.len_nodes() {
let idx = graph::NodeIndex(self.node_indices.len());
self.visit_one(idx);
} else {
for idx in 0..self.node_indices.len() {
if self.node_indices[idx].is_none() {
let idx = graph::NodeIndex(idx);
self.visit_one(idx);
break;
}
}
}
self.get_next();
}

if self.current_scc.is_empty() {
None
} else {
Some(self.current_scc.clone())
}
}
}
2 changes: 1 addition & 1 deletion src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ pub mod diagnostics;

pub mod build;
pub mod def_use;
pub mod callgraph;
pub mod graphviz;
mod hair;
pub mod mir_map;
pub mod pretty;
pub mod transform;

Loading