Skip to content

Commit

Permalink
Rollup merge of #132346 - nnethercote:some-graphviz-tweaks, r=cjgillot
Browse files Browse the repository at this point in the history
Some graphviz tweaks

r? `@cjgillot`
  • Loading branch information
jieyouxu authored Oct 30, 2024
2 parents c22832f + d921be9 commit 18e44f8
Showing 1 changed file with 58 additions and 53 deletions.
111 changes: 58 additions & 53 deletions compiler/rustc_mir_dataflow/src/framework/graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ pub(crate) struct Formatter<'mir, 'tcx, A>
where
A: Analysis<'tcx>,
{
body: &'mir Body<'tcx>,
results: RefCell<Option<Results<'tcx, A>>>,
// The `RefCell` is used because `<Formatter as Labeller>::node_label`
// takes `&self`, but it needs to modify the cursor. This is also the
// reason for the `Formatter`/`BlockFormatter` split; `BlockFormatter` has
// the operations that involve the mutation, i.e. within the `borrow_mut`.
cursor: RefCell<ResultsCursor<'mir, 'tcx, A>>,
style: OutputStyle,
reachable: BitSet<BasicBlock>,
}
Expand All @@ -48,11 +51,15 @@ where
style: OutputStyle,
) -> Self {
let reachable = mir::traversal::reachable_as_bitset(body);
Formatter { body, results: Some(results).into(), style, reachable }
Formatter { cursor: results.into_results_cursor(body).into(), style, reachable }
}

fn body(&self) -> &'mir Body<'tcx> {
self.cursor.borrow().body()
}

pub(crate) fn into_results(self) -> Results<'tcx, A> {
self.results.into_inner().unwrap()
self.cursor.into_inner().into_results()
}
}

Expand Down Expand Up @@ -81,7 +88,7 @@ where
type Edge = CfgEdge;

fn graph_id(&self) -> dot::Id<'_> {
let name = graphviz_safe_def_name(self.body.source.def_id());
let name = graphviz_safe_def_name(self.body().source.def_id());
dot::Id::new(format!("graph_for_def_id_{name}")).unwrap()
}

Expand All @@ -90,20 +97,11 @@ where
}

fn node_label(&self, block: &Self::Node) -> dot::LabelText<'_> {
let mut label = Vec::new();
self.results.replace_with(|results| {
// `Formatter::result` is a `RefCell<Option<_>>` so we can replace
// the value with `None`, move it into the results cursor, move it
// back out, and return it to the refcell wrapped in `Some`.
let mut fmt = BlockFormatter {
results: results.take().unwrap().into_results_cursor(self.body),
style: self.style,
bg: Background::Light,
};
let mut cursor = self.cursor.borrow_mut();
let mut fmt =
BlockFormatter { cursor: &mut cursor, style: self.style, bg: Background::Light };
let label = fmt.write_node_label(*block).unwrap();

fmt.write_node_label(&mut label, *block).unwrap();
Some(fmt.results.into_results())
});
dot::LabelText::html(String::from_utf8(label).unwrap())
}

Expand All @@ -112,20 +110,20 @@ where
}

fn edge_label(&self, e: &Self::Edge) -> dot::LabelText<'_> {
let label = &self.body[e.source].terminator().kind.fmt_successor_labels()[e.index];
let label = &self.body()[e.source].terminator().kind.fmt_successor_labels()[e.index];
dot::LabelText::label(label.clone())
}
}

impl<'mir, 'tcx, A> dot::GraphWalk<'mir> for Formatter<'mir, 'tcx, A>
impl<'tcx, A> dot::GraphWalk<'_> for Formatter<'_, 'tcx, A>
where
A: Analysis<'tcx>,
{
type Node = BasicBlock;
type Edge = CfgEdge;

fn nodes(&self) -> dot::Nodes<'_, Self::Node> {
self.body
self.body()
.basic_blocks
.indices()
.filter(|&idx| self.reachable.contains(idx))
Expand All @@ -134,10 +132,10 @@ where
}

fn edges(&self) -> dot::Edges<'_, Self::Edge> {
self.body
.basic_blocks
let body = self.body();
body.basic_blocks
.indices()
.flat_map(|bb| dataflow_successors(self.body, bb))
.flat_map(|bb| dataflow_successors(body, bb))
.collect::<Vec<_>>()
.into()
}
Expand All @@ -147,20 +145,20 @@ where
}

fn target(&self, edge: &Self::Edge) -> Self::Node {
self.body[edge.source].terminator().successors().nth(edge.index).unwrap()
self.body()[edge.source].terminator().successors().nth(edge.index).unwrap()
}
}

struct BlockFormatter<'mir, 'tcx, A>
struct BlockFormatter<'a, 'mir, 'tcx, A>
where
A: Analysis<'tcx>,
{
results: ResultsCursor<'mir, 'tcx, A>,
cursor: &'a mut ResultsCursor<'mir, 'tcx, A>,
bg: Background,
style: OutputStyle,
}

impl<'mir, 'tcx, A> BlockFormatter<'mir, 'tcx, A>
impl<'tcx, A> BlockFormatter<'_, '_, 'tcx, A>
where
A: Analysis<'tcx>,
A::Domain: DebugWithContext<A>,
Expand All @@ -173,7 +171,9 @@ where
bg
}

fn write_node_label(&mut self, w: &mut impl io::Write, block: BasicBlock) -> io::Result<()> {
fn write_node_label(&mut self, block: BasicBlock) -> io::Result<Vec<u8>> {
use std::io::Write;

// Sample output:
// +-+-----------------------------------------------+
// A | bb4 |
Expand All @@ -200,6 +200,9 @@ where
// attributes. Make sure to test the output before trying to remove the redundancy.
// Notably, `align` was found to have no effect when applied only to <table>.

let mut v = vec![];
let w = &mut v;

let table_fmt = concat!(
" border=\"1\"",
" cellborder=\"1\"",
Expand All @@ -219,21 +222,21 @@ where

// C: State at start of block
self.bg = Background::Light;
self.results.seek_to_block_start(block);
let block_start_state = self.results.get().clone();
self.cursor.seek_to_block_start(block);
let block_start_state = self.cursor.get().clone();
self.write_row_with_full_state(w, "", "(on start)")?;

// D + E: Statement and terminator transfer functions
self.write_statements_and_terminator(w, block)?;

// F: State at end of block

let terminator = self.results.body()[block].terminator();
let terminator = self.cursor.body()[block].terminator();

// Write the full dataflow state immediately after the terminator if it differs from the
// state at block entry.
self.results.seek_to_block_end(block);
if self.results.get() != &block_start_state || A::Direction::IS_BACKWARD {
self.cursor.seek_to_block_end(block);
if self.cursor.get() != &block_start_state || A::Direction::IS_BACKWARD {
let after_terminator_name = match terminator.kind {
mir::TerminatorKind::Call { target: Some(_), .. } => "(on unwind)",
_ => "(on end)",
Expand All @@ -250,8 +253,8 @@ where
match terminator.kind {
mir::TerminatorKind::Call { destination, .. } => {
self.write_row(w, "", "(on successful return)", |this, w, fmt| {
let state_on_unwind = this.results.get().clone();
this.results.apply_custom_effect(|analysis, state| {
let state_on_unwind = this.cursor.get().clone();
this.cursor.apply_custom_effect(|analysis, state| {
analysis.apply_call_return_effect(
state,
block,
Expand All @@ -265,18 +268,18 @@ where
colspan = this.style.num_state_columns(),
fmt = fmt,
diff = diff_pretty(
this.results.get(),
this.cursor.get(),
&state_on_unwind,
this.results.analysis()
this.cursor.analysis()
),
)
})?;
}

mir::TerminatorKind::Yield { resume, resume_arg, .. } => {
self.write_row(w, "", "(on yield resume)", |this, w, fmt| {
let state_on_coroutine_drop = this.results.get().clone();
this.results.apply_custom_effect(|analysis, state| {
let state_on_coroutine_drop = this.cursor.get().clone();
this.cursor.apply_custom_effect(|analysis, state| {
analysis.apply_call_return_effect(
state,
resume,
Expand All @@ -290,9 +293,9 @@ where
colspan = this.style.num_state_columns(),
fmt = fmt,
diff = diff_pretty(
this.results.get(),
this.cursor.get(),
&state_on_coroutine_drop,
this.results.analysis()
this.cursor.analysis()
),
)
})?;
Expand All @@ -302,8 +305,8 @@ where
if !targets.is_empty() =>
{
self.write_row(w, "", "(on successful return)", |this, w, fmt| {
let state_on_unwind = this.results.get().clone();
this.results.apply_custom_effect(|analysis, state| {
let state_on_unwind = this.cursor.get().clone();
this.cursor.apply_custom_effect(|analysis, state| {
analysis.apply_call_return_effect(
state,
block,
Expand All @@ -317,9 +320,9 @@ where
colspan = this.style.num_state_columns(),
fmt = fmt,
diff = diff_pretty(
this.results.get(),
this.cursor.get(),
&state_on_unwind,
this.results.analysis()
this.cursor.analysis()
),
)
})?;
Expand All @@ -328,7 +331,9 @@ where
_ => {}
};

write!(w, "</table>")
write!(w, "</table>")?;

Ok(v)
}

fn write_block_header_simple(
Expand Down Expand Up @@ -407,9 +412,9 @@ where
block: BasicBlock,
) -> io::Result<()> {
let diffs = StateDiffCollector::run(
self.results.body(),
self.cursor.body(),
block,
self.results.mut_results(),
self.cursor.mut_results(),
self.style,
);

Expand All @@ -420,7 +425,7 @@ where
if A::Direction::IS_FORWARD { it.next().unwrap() } else { it.next_back().unwrap() }
};

for (i, statement) in self.results.body()[block].statements.iter().enumerate() {
for (i, statement) in self.cursor.body()[block].statements.iter().enumerate() {
let statement_str = format!("{statement:?}");
let index_str = format!("{i}");

Expand All @@ -442,7 +447,7 @@ where
assert!(diffs_after.is_empty());
assert!(diffs_before.as_ref().map_or(true, ExactSizeIterator::is_empty));

let terminator = self.results.body()[block].terminator();
let terminator = self.cursor.body()[block].terminator();
let mut terminator_str = String::new();
terminator.kind.fmt_head(&mut terminator_str).unwrap();

Expand Down Expand Up @@ -492,8 +497,8 @@ where
mir: &str,
) -> io::Result<()> {
self.write_row(w, i, mir, |this, w, fmt| {
let state = this.results.get();
let analysis = this.results.analysis();
let state = this.cursor.get();
let analysis = this.cursor.analysis();

// FIXME: The full state vector can be quite long. It would be nice to split on commas
// and use some text wrapping algorithm.
Expand Down

0 comments on commit 18e44f8

Please sign in to comment.